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

pub publish -n is treating common infos as fatal #3870

Closed
fulpismo opened this issue Apr 11, 2023 · 9 comments · Fixed by #3877
Closed

pub publish -n is treating common infos as fatal #3870

fulpismo opened this issue Apr 11, 2023 · 9 comments · Fixed by #3877
Assignees
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@fulpismo
Copy link

fulpismo commented Apr 11, 2023

hello!
i have a concern regarding validating publishes. when using flutter pub publish -n the command runs an analyzer that has the --fatal-infos flag.
for scenarios where i have many infos that could not be treated at the moment (such as deprecation infos) i would end up stuck, because my validation fails. for infos, warnings and errors, the command returns an exit code 65.
is this expected? shouldn't there be a way to enable infos to be accepted? or to at least return a different type of response if there is different types of errors?
thanks!

@sigurdm
Copy link
Contributor

sigurdm commented Apr 11, 2023

You still only end up with a warning from dart pub publish meaning you can still force a publish.

You can also add // ignore: lines to ignore the specific infos.

I think it makes sense that pub publish complains if dart analyze is not "clean", as there should always be a way to get to that state.

@mugbug
Copy link

mugbug commented Apr 11, 2023

@sigurdm Wasn't it an option to expose the dart analyze --fatal-infos and --[no-]fatal-warnings flags instead of hardcode --fatal-infos?

@sigurdm
Copy link
Contributor

sigurdm commented Apr 11, 2023

Where would you expose it?

@mugbug
Copy link

mugbug commented Apr 11, 2023

As a flag to the pub publish command. For instance:

  • dart pub publish -n would only fail for errors and warnings
  • dart pub publish -n --no-fatal-warnings would only fail for errors
  • dart pub publish -n --fatal-infos would fail for infos, warnings and errors

Just like dart analyze works. Does that make sense?

I can't see a clear reason why "blocking" a package to be published just because some internal code isn't formatted according to some lint rules.

@sigurdm
Copy link
Contributor

sigurdm commented Apr 12, 2023

As a flag to the pub publish command.

That would become a configuration nightmare. I would rather just drop --fatal-infos, but for most projects getting to a "clean" dart analyze state should be both achievable and desirable. Especially if you are going to publish the result, doing the last bit of polish seems fair to expect.

I can't see a clear reason why "blocking" a package to be published just because some internal code isn't formatted according to some lint rules.

We are not blocking, the package from being published, merely giving a warning. There are ways of configuring the analyzer, either through ignore lines or [analysis_options.yaml] (https://dart.dev/guides/language/analysis-options).

@mugbug
Copy link

mugbug commented Apr 12, 2023

I've got your point and agree that a clean dart analyze should definitely be desirable. The only feedback I'd like to leave then is that this came as some sort of "breaking change" on our internal projects when bumping the dart version. I think this could've come as a clear breaking change on release notes. It was really hard to identify if the issues we were facing were related to something wrong that we did or if indeed there was some change on how publish works.

Not sure if we've searched on the right places, but we took a look on dart and flutter's release notes.

And thanks for the quick replies and explanations!

@sigurdm
Copy link
Contributor

sigurdm commented Apr 12, 2023

I can empathize with that. Sorry for breaking your flow.

This has been announced on the sdk changelog: https://github.com/dart-lang/sdk/blob/main/CHANGELOG.md#pub-1 (12th bullet-point). But it was not highlighted as a breaking change.

I don't think we usually think of new publish-validations as breaking changes, but maybe we really should. @jonasfj do you have an opinion here?

Using dart pub publish -n in CI is not super well supported, and we want to have some mechanism for opting out of each validation, but I don't think we have a design yet. (Perhaps you can include something in pubspec.yaml to acknowledge/ignore a warning) see: #3807

@mugbug
Copy link

mugbug commented Apr 12, 2023

Understood! But yeah, in case of possible breaking changes for publish-validation flows I think it would be really time-saving for many people to have the changes pointed out as breaking changes so it's easier to figure out why things broke when the dart version is updated, for instance.

Currently we are already working on fixing the infos for our packages, but it might be useful for other people as well as for future changes on pub commands.

@sigurdm
Copy link
Contributor

sigurdm commented Apr 13, 2023

We have agreed to remove the --fatal-infos flag. It's better to follow the analyzer here.

@sigurdm sigurdm added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Apr 13, 2023
@sigurdm sigurdm self-assigned this Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants