-
Notifications
You must be signed in to change notification settings - Fork 11
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
support sciml blue and yas styleguides #440
base: main
Are you sure you want to change the base?
Conversation
@abelsiqueira I marked this as draft as I need to update/fix tests, but feedback on the general direction welcome already. What do you think about making the question about indentation conditional? I think it's a bit more coherent, since if someone wants to pick a styleguide, might be better for the generator not to encourage them immediately to introduce exceptions. Another question, I think the linting will fail, because when I ran it locally it was sorting the lines of
into the nonsensical
is there a way to prevent this? |
e32250c
to
2b9214d
Compare
Thanks for the PR! The Indentation is used in other places - it also serves as the indentation of yaml, json and markdown (I think) files. So it should still be a required question. I don't use the styles, so I'm not sure if people follow them fully normally. It might be worth checking some random .JuliaFormatter.toml in the wild. Since I like indent 2, if I were to follow a style, I would probably still try to change it - i.e., add the indent even is style is selected. But we don't have to accommodate everything in the template, only the most expected (or recommended) practice. So probably it is fine to only have PS. I checked a few random files on GitHub searching for The sorting issue was unexpected, but I found an easy fix. The |
@@ -10,6 +10,7 @@ | |||
[![Coverage](https://codecov.io/gh/{{ PackageOwner }}/{{ PackageName }}.jl/branch/main/graph/badge.svg)](https://codecov.io/gh/{{ PackageOwner }}/{{ PackageName }}.jl) | |||
[![DOI](https://zenodo.org/badge/DOI/FIXME)](https://doi.org/FIXME) | |||
{% if AddCodeOfConduct %}[![Contributor Covenant](https://img.shields.io/badge/Contributor%20Covenant-2.1-4baaaa.svg)](CODE_OF_CONDUCT.md){% endif %} | |||
{% if StyleGuide == 'sciml' %}[![SciML Code Style](https://img.shields.io/static/v1?label=code%20style&message=SciML&color=9558b2&labelColor=389826)](https://github.com/SciML/SciMLStyle){% endif %}{% if StyleGuide == 'blue' %}[![Code Style: Blue](https://img.shields.io/badge/code%20style-blue-4495d1.svg)](https://github.com/JuliaDiff/BlueStyle){% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can keep them in separate lines. It should be possible to remove the empty spaces if we use {%-
and/or -%}
in either or both if
and endif
- yes, 2^4 combinations, because I don't understand how the whitespace removal works yet. Docs are here: https://jinja.palletsprojects.com/en/3.0.x/templates/#whitespace-control
I would try {%- if ... %} ... {%- endif %}
first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked on stack overflow, and the solution for this is to use
{% if stuff -%}
Badge
{% endif -%}
But I will work on this later on #445, so it's fine either way.
makes sense, I also find 2 spaces indentation more logical for yaml and other files. This decoupling allows to move the question about styleguide to I'll do the changes and look into the tests later today |
Yeah, I think I wanted to select indentation before deciding on strategy, because it will appear in the file regardless of strategy. But it makes more sense to have the style questions in code quality. I think we can move all the indentation questions there. I also realised that if we split the Indentation, we should also adjust If you think it is easier you can make a separate PR before this one only to split Indentation and move to code-quality. |
Just a heads up, I've added a |
Co-authored-by: Abel Soares Siqueira <[email protected]>
c488d85
to
4baf40a
Compare
PR changes summary:
Related issues
Closes #424
Checklist
I am following the contributing guidelines
Tests are passing
Lint workflow is passing
Docs were updated and workflow is passing
CHANGELOG.md was updated