feat: Format fails build in most sensible configurations

[romanr]

Status

READY

Description

Dart Format fails build in most sensible configurations

Type of Change

• ✨ New feature (non-breaking change which adds functionality)
• 🛠️ Bug fix (non-breaking change which fixes an issue)
• ❌ Breaking change (fix or feature that would cause existing functionality to change)
• 🧹 Code refactor
• ✅ Build configuration change
• 📝 Documentation
• 🗑️ Chore

[tomarra]

Hi @romanr, thanks for opening this PR.

After talking with the team on this we are running short on why this would be a useful change. Overall we want all code being submitted to follow a similar format for consistency and best practices. The Dart formatter can be updated to ignore certain rules if your looking for a way to get around errors being thrown which feels like the better usage pattern here.

Do you have a specific use case that your trying to achieve here that you can describe?

[romanr]

It fails the whole build if there is a single character formatting issue in any of the files. Why can't we have warning configurable.
Yes, it should work ideally when every single team member has synchronised formatter settings that work, and I congratulate you on having such a team. No sarcasm.

But you may commit text resource files, and it fail due to a non-code file. You have to spend time learning about the formatter, what to change, and where. What if developers use different IDEs and settings? There will be differences.

Formatting is not critical to performance; it is about code readability and maintainability. Not everyone works in a high-security facility with strict code reviews. Different teams have different work styles, and I sometimes have projects where I work alone, which makes formatting even less relevant.
Also, the question arises if you need to fix something urgently by editing in GitHub web; that would be akin to the movie Man on Wire with the formatting.

After we renamed a module, spelling check now fails the whole build! I crawled over that workflow file and couldn't find a solution, so I just turn that file off. Why does it have to fail and not give a warning? It's the definition of a spelling police.
What if you have an international team and people make spelling mistakes almost every day? It would be quite costly to run around and fix them one by one instead of reviewing all the spelling warnings once in a while.

I suppose you can keep it as rigid as you like if this is just for yourself and for those who remember how to configure the formatter and spell exceptions, in a beautiful world, I wish I was special.

I just want to ask how to get workflow to work in a fork, because after I pointed our CI to the fork, I am getting "no such parameter" for parameter that I added. Looks like commit is not part of "@v1"; there's a tagging script, but can I just add some tag to make it work?

[wolfenrain]

It fails the whole build if there is a single character formatting issue in any of the files. Why can't we have warning configurable. Yes, it should work ideally when every single team member has synchronised formatter settings that work, and I congratulate you on having such a team. No sarcasm.

But you may commit text resource files, and it fail due to a non-code file. You have to spend time learning about the formatter, what to change, and where. What if developers use different IDEs and settings? There will be differences.

Formatting is not critical to performance; it is about code readability and maintainability. Not everyone works in a high-security facility with strict code reviews. Different teams have different work styles, and I sometimes have projects where I work alone, which makes formatting even less relevant. Also, the question arises if you need to fix something urgently by editing in GitHub web; that would be akin to the movie Man on Wire with the formatting.

After we renamed a module, spelling check now fails the whole build! I crawled over that workflow file and couldn't find a solution, so I just turn that file off. Why does it have to fail and not give a warning? It's the definition of a spelling police. What if you have an international team and people make spelling mistakes almost every day? It would be quite costly to run around and fix them one by one instead of reviewing all the spelling warnings once in a while.

I suppose you can keep it as rigid as you like if this is just for yourself and for those who remember how to configure the formatter and spell exceptions, in a beautiful world, I wish I was special.

I just want to ask how to get workflow to work in a fork, because after I pointed our CI to the fork, I am getting "no such parameter" for parameter that I added. Looks like commit is not part of "@v1"; there's a tagging script, but can I just add some tag to make it work?

The workflows are how we, VGV, apply our standards to code both internally and externally, they might not fit everyone’s workflow. We prefer our workflows to have flags that control formatting, so a "disabling formatting" flag would not fit with our own standards and imho is a disaster to happen.

You can as of course fork and change it locally, as you mentioned already. You can easily refer to forked workflows by their GitHub path and commit/branch or tag:

uses: <YourOrgOrUser>/very_good_workflows/.github/workflows/dart_package.yml@<tag|branch|commit_sha>

See the GitHub Workflow syntax for more information.

PS: the Dart team is working on new ways to change Dart formatting (dart-lang/sdk#56863) which imho would be the preferred way for controlling dart formatting, in theory one could then disable things in the config so the CI would just follow that instead of having to pass them as flags to the CI.

[romanr]

It fails the whole build if there is a single character formatting issue in any of the files. Why can't we have warning configurable. Yes, it should work ideally when every single team member has synchronised formatter settings that work, and I congratulate you on having such a team. No sarcasm.
But you may commit text resource files, and it fail due to a non-code file. You have to spend time learning about the formatter, what to change, and where. What if developers use different IDEs and settings? There will be differences.
Formatting is not critical to performance; it is about code readability and maintainability. Not everyone works in a high-security facility with strict code reviews. Different teams have different work styles, and I sometimes have projects where I work alone, which makes formatting even less relevant. Also, the question arises if you need to fix something urgently by editing in GitHub web; that would be akin to the movie Man on Wire with the formatting.
After we renamed a module, spelling check now fails the whole build! I crawled over that workflow file and couldn't find a solution, so I just turn that file off. Why does it have to fail and not give a warning? It's the definition of a spelling police. What if you have an international team and people make spelling mistakes almost every day? It would be quite costly to run around and fix them one by one instead of reviewing all the spelling warnings once in a while.
I suppose you can keep it as rigid as you like if this is just for yourself and for those who remember how to configure the formatter and spell exceptions, in a beautiful world, I wish I was special.
I just want to ask how to get workflow to work in a fork, because after I pointed our CI to the fork, I am getting "no such parameter" for parameter that I added. Looks like commit is not part of "@v1"; there's a tagging script, but can I just add some tag to make it work?

The workflows are how we, VGV, apply our standards to code both internally and externally, they might not fit everyone’s workflow. We prefer our workflows to have flags that control formatting, so a "disabling formatting" flag would not fit with our own standards and imho is a disaster to happen.

You can as of course fork and change it locally, as you mentioned already. You can easily refer to forked workflows by their GitHub path and commit/branch or tag:

uses: <YourOrgOrUser>/very_good_workflows/.github/workflows/dart_package.yml@<tag|branch|commit_sha>

See the GitHub Workflow syntax for more information.

PS: the Dart team is working on new ways to change Dart formatting (dart-lang/sdk#56863) which imho would be the preferred way for controlling dart formatting, in theory one could then disable things in the config so the CI would just follow that instead of having to pass them as flags to the CI.

I made it off by default as suggested. still no?

[tomarra]

I made it off by default as suggested. still no?

Hi @romanr. After talking more with the team I'm going to close this PR out. This isn't something that we plan on supporting as it doesn't hold to our standards & practices. Like stated by @wolfenrain you can easily make this change in your fork and point your projects to use that workflow if you need this change. I also suggest adding in some comments on the linked Dart issue (dart-lang/sdk#56863) as really that refactor would be the best outcome for everyone in making the formatter a bit more config driven.