invertase / melos

๐ŸŒ‹ A tool for managing Dart projects with multiple packages. With IntelliJ and Vscode IDE support. Supports automated versioning, changelogs & publishing via Conventional Commits.
https://melos.invertase.dev/~melos-latest
Apache License 2.0
1.08k stars 193 forks source link

feat: built-in melos command for analyzing projects #655

Closed jessicatarra closed 4 months ago

jessicatarra commented 4 months ago

Description

First things first, I'm really excited to work on this issue because this year I made a personal goal to contribute more to open source projects. It makes me happy to be able to do it for a tool that I use every day.

Usually, melos projects use a script to run either dart analyze or flutter analyze, as mentioned by @jpnurmi in issue #424. However, I agree that it would be better to have a built-in command for this. Some newbies to the tool might try running this command and then realize they need extra setup to get the same results.

So, this pull request aims to address this problem by adding the needed functionality. It also includes features like running tasks concurrently, filtering, and support for --fatal-infos and --[no-]fatal-warnings options.

I also took the time to update the readme file and provide the latest help section, as well as update the melos.yaml file and remove the old way to run the analyzer.

Dear maintainers, please feel free to review, modify, or add code to this contribution :)

Type of Change

Closes #424

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

jessicatarra commented 4 months ago

Yes sure, I just added the proper documentation for the analyzer command @spydon. Commit: 963b7022a1cde055da319fe8c71370ad89ec3b55

spydon commented 4 months ago

I wonder if we might need to treat this as a breaking change, even though it technically isn't one, because so many of our users have a script called analyze. What happens if the user has a script called analyze already and runs melos analyze with the new version?

jessicatarra commented 4 months ago

It's a good question, so I did some tests, and it seems that running just melos analyze will trigger the built-in command, while running melos run analyze will execute the script. I'm also aware that you can call a script without using the run keyword. However, if the user is specifically using the version with the analyzer command, it will fall into the first situation I mentioned earlier. What we can do, and this is my suggestion, is to let the user know that there's a better way to trigger the analyzer when they attempt to use it through the script, what do you think?

spydon commented 4 months ago

and this is my suggestion, is to let the user know that there's a better way to trigger the analyzer when they attempt to use it through the script, what do you think?

Yeah, that sounds good. Most users will probably not even notice that would be using the built-in command instead of their own after they have updated, until they read about it. Since we don't fail fatally we'll hopefully not crash everyone's pipelines, so I think we're fine. :sweat_smile:

Salakar commented 4 months ago

Very nice! Thanks @jessicatarra for doing this ๐ŸŽ‰

renefloor commented 3 months ago

Most users will probably not even notice that would be using the built-in command instead of their own after they have updated, until they read about it.

@spydon I would have preferred melos to block the analyze command if it's duplicated in the melos.yaml. Now our CI started to fail and it was not immediately clear why. It's just silently overwriting our own command and not giving any warning about that.

spydon commented 3 months ago

@spydon I would have preferred melos to block the analyze command if it's duplicated in the melos.yaml. Now our CI started to fail and it was not immediately clear why. It's just silently overwriting our own command and not giving any warning about that.

That's why we did a major version bump, since you then have to actively go into your root pubspec and bump Melos to the next major version before this comes into effect. But you can open a feature request for allowing analyze and format to be overwritten.