Skip to content

Conversation

sgaist
Copy link
Contributor

@sgaist sgaist commented Dec 4, 2022

This merge request implements a couple of requested format support.

Each format is in it's own commit to keep things clearly separated.

Fixes #575
Fixes #614
Fixes #616

@nicorikken
Copy link
Member

@sgaist Thanks a lot! Reading up on .clang-format I see this is supposed to be the entire filename, not the extension. Also the _clang-format filename is a valid alternative. I added it in #575 perhaps you can add that to that commit? Otherwise I think this looks good.

@sgaist
Copy link
Contributor Author

sgaist commented Dec 4, 2022

Sure thing !

Should we consider adding support for other tools configuration files like .flake8, .bandit and the likes ?

@sgaist sgaist force-pushed the new_format_support branch from 1267e28 to 3d9add8 Compare December 4, 2022 20:58
@nicorikken
Copy link
Member

If you have suggestions, please do. Please provide some examples or references in the commit or discussion so we can refer to it at a later stage.

@sgaist
Copy link
Contributor Author

sgaist commented Dec 4, 2022

Shouldn't I rather open an issue about that topic so we can discuss there the list of tools ?

@nicorikken
Copy link
Member

@sgaist yeah that would probably be best.

@sgaist
Copy link
Contributor Author

sgaist commented Dec 6, 2022

In this case, should I remove the .clang-format commit from this PR and put it back when the tools file support discussion is done ?

@carmenbianca
Copy link
Member

You can put it in FILENAME_COMMENT_STYLE_MAP.

_clang-format I am not sure about.

@sgaist
Copy link
Contributor Author

sgaist commented Dec 6, 2022

Thanks @carmenbianca I somehow missed that one. Will be perfect for #633.

@sgaist sgaist force-pushed the new_format_support branch from 3d9add8 to 6b773a9 Compare December 7, 2022 09:16
Copy link
Member

@carmenbianca carmenbianca left a comment

Choose a reason for hiding this comment

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

LGTM

@sgaist
Copy link
Contributor Author

sgaist commented Feb 7, 2023

One thing I just realized, I have listed the specific issues per commit which can trigger quite a lot of noise on their respective page.

I think I should rewrite the message of these commits and only mention the issues in the pull request.

Agreed ?

@sgaist sgaist force-pushed the new_format_support branch from 9bd77fc to 777f775 Compare February 7, 2023 09:53
@sgaist
Copy link
Contributor Author

sgaist commented Feb 7, 2023

I fixed the pylint issue and did the message change at the same time.

@carmenbianca
Copy link
Member

Added a change log. Will merge when tests succeed. Thank you @sgaist !

sgaist and others added 4 commits February 8, 2023 23:17
.mjs is already present
This should also cover the proguard-rules.pro file mentionned in
ticket 614
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
@carmenbianca carmenbianca merged commit f42e729 into fsfe:main Feb 8, 2023
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.

Support for .cjs and .mjs files Comment style for proguard rules reuse should detect .clang-format files
4 participants