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

Lint: Update rules for formating code #414

Merged
merged 2 commits into from
Jan 31, 2024

Conversation

ljubon
Copy link
Contributor

@ljubon ljubon commented Jan 31, 2024

Based on existing rule sets in ParquetSharp.DotSettings and as part of OSS-414 goal of this PR is to add new rules which are safe and not breaking current process, just enforcing existing syntax and style

Optional: I tried one more rule to add, but it's not part of this PR
Here are changes , total 67 file
Rule line is:

<s:Boolean x:Key="/Default/CodeStyle/CodeFormatting/CSharpFormat/SPACE_AFTER_TYPECAST_PARENTHESES/@EntryValue">False</s:Boolean>

@adamreeve
Copy link
Contributor

I'm not sure about the SPACE_AFTER_TYPECAST_PARENTHESES change. I prefer the space there, but it looks like no space is the default and the standard that a lot of C# projects follow.

I don't feel too strongly either way, but if we want to keep the spaces, then we should add that check but set it to true rather than false.

@ljubon
Copy link
Contributor Author

ljubon commented Jan 31, 2024

I'm not sure about the SPACE_AFTER_TYPECAST_PARENTHESES change. I prefer the space there, but it looks like no space is the default and the standard that a lot of C# projects follow.

I don't feel too strongly either way, but if we want to keep the spaces, then we should add that check but set it to true rather than false.

Would you like me to set it to false for SPACE_AFTER_TYPECAST_PARENTHESES and run that check locally in order to see how big change would it be?

@adamreeve
Copy link
Contributor

Yes that would be helpful thanks, although I guess you mean set it to true?

@ljubon
Copy link
Contributor Author

ljubon commented Jan 31, 2024

FYI @adamreeve it seems that SPACE_AFTER_TYPECAST_PARENTHESES set to true doesn't affect current files, PR is updated

Example output: https://github.com/ljubon/ParquetSharp/actions/runs/7732127206/job/21081346074

@adamreeve
Copy link
Contributor

Cool, thanks @ljubon, I'm happy with that

@adamreeve adamreeve merged commit bfe314f into G-Research:master Jan 31, 2024
30 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.

None yet

3 participants