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

Reinstate lost style checks #1591

Merged
merged 3 commits into from
Feb 27, 2024
Merged

Conversation

mosteo
Copy link
Member

@mosteo mosteo commented Feb 26, 2024

It seems the changes in #1497 removed quite more style checks than intended. In regard to @simonjwright concerns in that PR, I think we can just disable that specific check once it is enabled by default in -gnatyg.

@mosteo mosteo marked this pull request as ready for review February 26, 2024 19:15
@mosteo mosteo added this to the 2.0 milestone Feb 26, 2024
@simonjwright
Copy link
Contributor

Personally I’ve spent approaching 25 years being completely happy with plain -gnaty. The mistake I made in #1497 was leaving out -gnatyy (standard style checks). Looks to me as though the problem could have been fixed just by including that.

We know -gnatyg now includes -gnatyz => "check parentheses not required by operator precedence rules", which craziness (IMO) was the prompt for #1497. I suppose we could achieve that by -gnaty-z? (after the -gnatyg, ofc, as you’ve done with -gnaty-s).

I would really like -gnaty-z. Just don’t include -gnatyo!

alire_common.gpr Outdated
"-gnatyu", -- no unnecessary blank lines
"-gnatyx", -- no extra parens around conditionals
"-gnaty-s"); -- relax fwd decl
"-gnatyg", -- Standard GNAT style = -gnatyydISuxz
Copy link
Member

Choose a reason for hiding this comment

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

One potential risk with gnatyg is that its definition can change in the future, which can bring some unwanted style errors.

Why not use our default style check switches for Alire crates?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I prefer to make an sporadic fix if that happens rather than being unaware of recommended style changes.

@mosteo
Copy link
Member Author

mosteo commented Feb 27, 2024

No love for -gnatyo here either. And also no problem with -gnaty-z.

@mosteo
Copy link
Member Author

mosteo commented Feb 27, 2024

But I'm seeing a different problem now which is that different compilers understand different switches, so it's better to go for a explicit list in the end, so I'll take Fabien's suggestion.

@mosteo mosteo merged commit d6c2572 into alire-project:master Feb 27, 2024
27 checks passed
@mosteo mosteo deleted the fix/style-checks branch February 27, 2024 11:45
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.

3 participants