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.

Sign up for GitHub

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

Jump to bottom

chore(clang-format): unify rdsn and Pegasus clang-format #1022

Merged
merged 2 commits into from
Jul 7, 2022
Merged

chore(clang-format): unify rdsn and Pegasus clang-format #1022

merged 2 commits into from
Jul 7, 2022

Conversation

acelyc111
Copy link
Member

@acelyc111 acelyc111 commented Jun 28, 2022 •
edited
Loading

What problem does this PR solve?

Unify the rdsn and the Pegasus projects' clang-format tools since they are duplicated.

What is changed and how does it work?

  1. move .clang-format to the root path
  2. update .github/workflows/lint_and_test_pegasus.yaml to lint all cpp files' style in Pegasus
  3. remove lint job from .github/workflows/test_rdsn.yaml, and rename workflow. Same to rdsn/README.md.
  4. update .licenserc.yaml, LICENSE, .rat-excludes since some license issues have been resolved, or files been removed.
  5. remove lint files from rdsn: rdsn/run.sh, rdsn/scripts/linux/run-clang-format.sh
  6. update lint files in Pegasus to lint all files in the repo: scripts/format_files.sh, scripts/run-clang-format.py
  7. remove outdated files: scripts/travis.sh

Checklist

The workflow work well as expect. I submmit a patch with some code with bad style will be reported: https://github.com/apache/incubator-pegasus/runs/7097185266?check_suite_focus=true

@acelyc111 acelyc111 force-pushed the unify_clang_format branch 2 times, most recently from 3c81a19 to 3973bee Compare June 28, 2022 16:12
@acelyc111 acelyc111 closed this Jun 28, 2022
@acelyc111 acelyc111 reopened this Jun 28, 2022
@acelyc111 acelyc111 closed this Jun 28, 2022
@acelyc111 acelyc111 reopened this Jun 28, 2022
levy5307
levy5307 previously approved these changes Jun 29, 2022
View reviewed changes
foreverneverer
foreverneverer previously approved these changes Jun 29, 2022
View reviewed changes
@acelyc111 acelyc111 dismissed stale reviews from foreverneverer and levy5307 via 7f070ff June 29, 2022 11:39
neverchanje
neverchanje previously approved these changes Jun 29, 2022
View reviewed changes
Copy link
Contributor

@neverchanje neverchanje left a comment

Choose a reason for hiding this comment

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

Lgtm

levy5307
levy5307 previously approved these changes Jun 30, 2022
View reviewed changes
@acelyc111 acelyc111 dismissed stale reviews from levy5307 and neverchanje via 53a12bf June 30, 2022 07:09
foreverneverer pushed a commit to foreverneverer/pegasus that referenced this pull request Jun 30, 2022
@levy5307
fix: stack overflow in duplication test (apache#1022)
fa58cd7
neverchanje
neverchanje approved these changes Jul 7, 2022
View reviewed changes
Copy link
Contributor

@neverchanje neverchanje left a comment

Choose a reason for hiding this comment

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

Can it be merged now?

@acelyc111
Copy link
Member Author

Can it be merged now?

It will report an illegal instruction error if I don't skip building third party, as shown in https://github.com/apache/incubator-pegasus/runs/7209009710?check_suite_focus=true, I'm not sure how it happened, even I re-trigger rebuild the third party bin package.

@acelyc111
Copy link
Member Author

Can it be merged now?

It will report an illegal instruction error if I don't skip building third party, as shown in https://github.com/apache/incubator-pegasus/runs/7209009710?check_suite_focus=true, I'm not sure how it happened, even I re-trigger rebuild the third party bin package.

So I remove --skip_thirdpary in .github/workflows/lint_and_test_pegasus.yaml

@acelyc111
Copy link
Member Author

Let's merge this pr, the left issue is not introduced by this pr, we can resolve it latter.

@acelyc111 acelyc111 merged commit a4dd5d3 into apache:master Jul 7, 2022
@acelyc111 acelyc111 mentioned this pull request Jul 11, 2022
Closed
@acelyc111
Copy link
Member Author

also related to: #1053

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neverchanje neverchanje neverchanje approved these changes

@foreverneverer foreverneverer foreverneverer approved these changes

@levy5307 levy5307 levy5307 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@acelyc111 @neverchanje @levy5307 @foreverneverer