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

Enforce editorconfig style by travis CI + fix style violations #127

Merged
merged 3 commits into from Sep 28, 2019
Merged

Enforce editorconfig style by travis CI + fix style violations #127

merged 3 commits into from Sep 28, 2019

Conversation

madebr
Copy link
Contributor

@madebr madebr commented Sep 25, 2019

  • Let Travis-CI enforce editorconfig rules (and fail CI when a violation is found)
  • Fix all current style violations:
    • I've moved gif.h and ym2151.[ch] to an external folder as these are files not owned by ths project (and keep the diff to the original small)
    • Ignore styles for the cpu and extern directory, as (most of) these files are included from external projects

Status before this pull request: https://travis-ci.com/madebr/x16-emulator/jobs/239039488
Status after this pull request: https://travis-ci.com/madebr/x16-emulator/jobs/239067490 (no violations)

@madebr
Copy link
Contributor Author

madebr commented Sep 28, 2019

Rebased on current master.

@mist64
Copy link
Collaborator

mist64 commented Sep 28, 2019

This looks like a good idea, but I have no experience with anything like this, so here's my question: Will this complain about the style of pull requests, so people offering a pull request will know what to change? Or will it only tell me what is wrong after I merge it?

@madebr
Copy link
Contributor Author

madebr commented Sep 28, 2019

@mist64 Yes, the check will be performed every time travis-ci performs a build, which is on every action on the git tree of this repository: a push to a branch, tagging of a commit, on every pull request, ...
The author of the pull request can check whether all tests are in the green and fix his pull request.
You as the maintainer of this project will always have the final decision on merge/no-merge.

@mist64 mist64 merged commit 1f618f2 into commanderx16:master Sep 28, 2019
@madebr madebr mentioned this pull request Sep 29, 2019
Open
@rsbohn12
Copy link
Contributor

Is it possible to have just one .editorconfig file?

@madebr
Copy link
Contributor Author

madebr commented Sep 29, 2019 •
edited

I created multiple configs because the files in extern and cpu have spaces instead of tabs.
I think another file tree can merge these.
e.g. put all extern sources in extern/... and all project specific sources in src/....
The .editorconfig can then specify tabs for all src/*.c and don't specify anything for sources in extern.
.editorconfig are meant to be used by editors, not for checking rules. clang-format might be something useful to look into.

fvdhoef pushed a commit that referenced this pull request Apr 17, 2020
@madebr @mist64
Enforce editorconfig style by travis CI + fix style violations (#127)
ae3be3f 
* Check .editorconfig rules in travis-ci + update .editorconfig

* Move ym2151 and gif.h to extern source folder

* Fix editorconfig violations + ignore cpu/extern subfolder
@madebr madebr deleted the editorconfig_check branch April 26, 2020 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@madebr @mist64 @rsbohn12