Skip to content

Reinstate lost style checks#1591

Merged
mosteo merged 3 commits into
alire-project:masterfrom
mosteo:fix/style-checks
Feb 27, 2024
Merged

Reinstate lost style checks#1591
mosteo merged 3 commits into
alire-project:masterfrom
mosteo:fix/style-checks

Conversation

@mosteo

@mosteo mosteo commented Feb 26, 2024

Copy link
Copy Markdown
Member

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
Copy Markdown
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!

Comment thread 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
Copy Markdown
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
Copy Markdown
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

mosteo commented Feb 27, 2024

Copy link
Copy Markdown
Member Author

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

@mosteo

mosteo commented Feb 27, 2024

Copy link
Copy Markdown
Member Author

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
@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