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

Auto-format, restructure project for CI #50

Merged
merged 2 commits into from
Mar 27, 2020
Merged

Auto-format, restructure project for CI #50

merged 2 commits into from
Mar 27, 2020

Conversation

woodruffw
Copy link
Member

Add a Linux CI target.

@woodruffw woodruffw mentioned this pull request Mar 27, 2020
7 tasks
@woodruffw woodruffw marked this pull request as ready for review March 27, 2020 20:40
@woodruffw woodruffw requested a review from ekilmer March 27, 2020 20:41
Copy link
Contributor

@ekilmer ekilmer left a comment

Choose a reason for hiding this comment

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

Other than the small nit, LGTM

Copy link
Contributor

@ekilmer ekilmer left a comment

Choose a reason for hiding this comment

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

Actually, we should add something in the README about using make lint to make sure code is formatted correctly.

Might be useful to add a make format that actually fixes the linter too.

@woodruffw
Copy link
Member Author

Actually, we should add something in the README about using make lint to make sure code is formatted correctly.

Will do!

Might be useful to add a make format that actually fixes the linter too.

This shouldn't be necessary, since make lint modifies the files in-place and git diff supplies the error code.

@haxmeadroom
Copy link
Contributor

Actually, we should add something in the README about using make lint to make sure code is formatted correctly.

Might be useful to add a make format that actually fixes the linter too.
Yeah, something we can run manually and sanity check.

Copy link
Contributor

@haxmeadroom haxmeadroom left a comment

Choose a reason for hiding this comment

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

Looks cool. Would prefer formatting to be manually done with a command for sanity checking.

@ekilmer
Copy link
Contributor

ekilmer commented Mar 27, 2020

This shouldn't be necessary, since make lint modifies the files in-place and git diff supplies the error code.

Ahhh, right. Haha forgot that clang-format doesn't have a check option, only in-place

@woodruffw
Copy link
Member Author

Looks cool. Would prefer formatting to be manually done with a command for sanity checking.

Yep, I've added a README note to that effect. make lint should work as expected locally!

Ahhh, right. Haha forgot that clang-format doesn't have a check option, only in-place

It actually does, but the check option doesn't return an exit code either. One day I'll submit that patch 😉.

@woodruffw woodruffw merged commit ac0710e into master Mar 27, 2020
@woodruffw woodruffw deleted the ww/more-ci branch March 27, 2020 21:01
@woodruffw woodruffw added this to the 2.0 milestone May 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants