Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.clang-format: Update to match coding conventions #1390

Merged
merged 1 commit into from
May 29, 2024

Conversation

tbunch1
Copy link
Contributor

@tbunch1 tbunch1 commented Apr 21, 2024

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.uk on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): [N/A]
  • Platform(s): [N/A]
  • Application(s): [N/A]

Additional configuration

N/A

Description of changes

Updates .clang-format to better reflect the Unikraft coding conventions.
Co-authored-by: Eddie Cazares ecazares15@utexas.edu
Co-authored-by: Lindsey Bowen lindseyb803@gmail.com

GitHub-Fixes: #975

@tbunch1 tbunch1 requested a review from a team as a code owner April 21, 2024 14:18
@tbunch1 tbunch1 changed the title Tbunch1/clang updates update .clang-format Apr 21, 2024
@tbunch1 tbunch1 changed the title update .clang-format .clang-format: Update to match coding conventions Apr 21, 2024
.clang-format Outdated Show resolved Hide resolved
@craciunoiuc
Copy link
Member

I think you should also merge all commits into a single one 🙏

@tbunch1 tbunch1 marked this pull request as draft April 25, 2024 22:50
@tbunch1
Copy link
Contributor Author

tbunch1 commented Apr 25, 2024

Apologies if this is a dumb question, but how should we do that? We're pretty new to contributing to open source and we can't figure out how to merge them together. I've been trying to rebase them but I'm having trouble because the changes are pushed. Again sorry if this is a simple question, but we're stuck

@craciunoiuc
Copy link
Member

No worries,

on your local machine run these commands:

  1. git rebase --interactive HEAD~3
  2. in the prompt change the last two pick to fixup and the first pick to reword
  3. change the first commit message to: .clang-format: Update to match coding conventions and continue
  4. git push --force

I hope this works/helps 😅

@github-actions github-actions bot added area/lib Internal Unikraft Microlibrary lang/c Issues or PRs to do with C/C++ labels Apr 26, 2024
@tbunch1
Copy link
Contributor Author

tbunch1 commented Apr 26, 2024

Oh I see. I was worried about messing with the commit that merged the recent updates but I guess I didn't understand how it worked.

@tbunch1 tbunch1 marked this pull request as ready for review April 26, 2024 22:15
@razvand razvand self-assigned this Apr 28, 2024
@razvand razvand requested a review from StefanJum April 28, 2024 05:34
@razvand razvand added this to the v0.17.0 (Calypso) milestone Apr 28, 2024
Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tbunch1 please remove the changes to pagefault64.c, since they are not part of this pr

@razvand
Copy link
Contributor

razvand commented May 21, 2024

@tbunch1 please remove the changes to pagefault64.c, since they are not part of this pr

@tbunch1 , could you please see the comments from @StefanJum and update the PR accordingly?

See also the checkpatch checks.

Updated .clang-format to match Unikraft coding conventions

Co-authored-by: Eddie Cazares <ecazares15@utexas.edu>
Co-authored-by: Lindsey Bowen <lindseyb803@gmail.com>
Signed-off-by: Thomas Bunch <tebunch@icloud.com>
Copy link
Member

@craciunoiuc craciunoiuc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it looks okay. Thanks!

Reviewed-by: Cezar Craciunoiu cezar.craciunoiu@unikraft.io

Copy link
Member

@StefanJum StefanJum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed-by: Stefan Jumarea stefanjumarea02@gmail.com

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved-by: Razvan Deaconescu razvand@unikraft.io

@razvand razvand added the merge Label to trigger merge action label May 29, 2024
@unikraft-bot unikraft-bot changed the base branch from staging to staging-1390 May 29, 2024 09:41
@unikraft-bot unikraft-bot merged commit a02a8a1 into unikraft:staging-1390 May 29, 2024
15 checks passed
unikraft-bot pushed a commit that referenced this pull request May 29, 2024
Updated .clang-format to match Unikraft coding conventions

Co-authored-by: Eddie Cazares <ecazares15@utexas.edu>
Co-authored-by: Lindsey Bowen <lindseyb803@gmail.com>
Signed-off-by: Thomas Bunch <tebunch@icloud.com>
Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.io>
Reviewed-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
GitHub-Closes: #1390
@unikraft-bot unikraft-bot added the ci/merged Merged by CI label May 29, 2024
@unikraft-bot unikraft-bot removed the merge Label to trigger merge action label May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/lib Internal Unikraft Microlibrary ci/merged Merged by CI lang/c Issues or PRs to do with C/C++
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

Update .clang-format (and .checkpatch.conf) according to the coding convention.
6 participants