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

Add Formatter (spec + CI step) #56

Merged
merged 4 commits into from
Dec 31, 2023
Merged

Conversation

svilupp
Copy link
Contributor

@svilupp svilupp commented Nov 27, 2023

Closes #55

This PR introduces 3 changes:

  • .JuliaFormatter.toml formatting spec set to SciML formatting guidelines
  • CI step (last step) that fails if PR is not formatted (to maintain style over time)
  • Minor update to CONTRIBUTING to reflect this change in the PR requirements

Why? It saves you time. You don't have to add spaces manually, you don't have to disabled your VSCode auto-formatter, etc. It all just works.

The code does not look pretty and sometimes it looks a bit suboptimal (too many lines), but it never impacts readability negatively.

Let me know what you think!

EDIT: the large amount of change is purely the effect of running the formatter. I haven't touched any files besides the ones outlined above.

@cpfiffer
Copy link
Collaborator

Can we bump this one?

@cpfiffer
Copy link
Collaborator

Seems like the failures here are due to not having an API key for the testing suite. I'd recommend we merge this in since the changes are cosmetic and the JuliaFormatter step seems appropriate.

@cpfiffer cpfiffer mentioned this pull request Dec 1, 2023
19 tasks
@roryl23
Copy link
Collaborator

roryl23 commented Dec 31, 2023

Happy to merge this as soon as the conflicts are resolved. I can resolve in the morning, almost kicked this illness

@svilupp
Copy link
Contributor Author

svilupp commented Dec 31, 2023

Hey! Sorry to hear you were sick... Let me refresh it with Main -- it's a simple formatter, so it shouldn't take any effort.

@roryl23 roryl23 merged commit c3f8b34 into JuliaML:main Dec 31, 2023
1 of 2 checks passed
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.

Add Formatter spec to make contributions easier
3 participants