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

Verify formatting in pipelines #10684

Merged
merged 13 commits into from May 17, 2023
Merged

Conversation

YohDeadfall
Copy link
Collaborator

@YohDeadfall YohDeadfall commented May 12, 2023

That pull requests adds formatting check to Azure pipelines, so reviewers won't spend their time on that task. Perhaps, it will require formatting the existing code, but maybe not.

Rules can be configured though .editoconfig.

Copy link
Contributor

@nopara73 nopara73 left a comment

Choose a reason for hiding this comment

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

cACK, code LGTM

yahiheb
yahiheb previously approved these changes May 13, 2023
Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

cACK

@molnard
Copy link
Collaborator

molnard commented May 15, 2023

I can add this, but GitHub action is easier and also the future. Can you try to do the same with GitHub Actions?

@YohDeadfall
Copy link
Collaborator Author

Sure, I just thought that you use Azure for a very strong reason.

@molnard
Copy link
Collaborator

molnard commented May 15, 2023

There was but it seems like not a problem anymore since we already added actions and nobody complained.

#2329 (comment)

Copy link
Contributor

@nopara73 nopara73 left a comment

Choose a reason for hiding this comment

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

David is right. We're moving to GitHub actions for a while now. This'd be a step in the wrong direction.

@YohDeadfall YohDeadfall requested a review from nopara73 May 16, 2023 14:01
Copy link
Collaborator

@molnard molnard left a comment

Choose a reason for hiding this comment

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

image

SadEyesPuppyEyesGIF

@YohDeadfall
Copy link
Collaborator Author

@molnard, well, isn't this PR intending to improve code quality? So, in addition to the pipeline I fixed all formatting issues we had (mostly spaces and usings) and a few CA2000 warnings. If I did all of these changes and checked them using git diff for each commit, I guess, you can do the same or at least review it. GitHub shows a Viewed check box for each file which can help you.

@YohDeadfall YohDeadfall requested a review from molnard May 16, 2023 15:17
@yahiheb yahiheb mentioned this pull request May 17, 2023
@yahiheb
Copy link
Collaborator

yahiheb commented May 17, 2023

To make the review of this PR easier for everyone I have cherry picked a couple of commits and I have opened the following PRs:
#10713, #10714, #10715, #10717

Please review/merge those first.

@nopara73 nopara73 merged commit a8c1f6e into zkSNACKs:master May 17, 2023
6 of 8 checks passed
@YohDeadfall YohDeadfall deleted the check-formating branch May 17, 2023 06:23
@molnard
Copy link
Collaborator

molnard commented May 17, 2023

Can you tell me why the formatting pipeline fails here?

#10711

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants