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.

Sign up for GitHub

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

Jump to bottom

[v7.0.0-dev.2] request: update melos analyze behaviour #827

Closed
1 task done
ricardodalarme opened this issue Jan 8, 2025 · 7 comments · Fixed by #829
Closed
1 task done

[v7.0.0-dev.2] request: update melos analyze behaviour #827

ricardodalarme opened this issue Jan 8, 2025 · 7 comments · Fixed by #829
Labels
feature request

Comments

@ricardodalarme
Copy link

ricardodalarme commented Jan 8, 2025 •
edited
Loading

Is there an existing feature request for this?

  • I have searched the existing issues.

Command

melos analyze

Description

Now with Pub Workspaces, commands like flutter pub get and flutter analyze can run across the entire workspace at once. This is significantly faster than the previous approach, where tools had to be run separately for each package. As a result, the current behavior of melos analyze, which runs dart analyze or flutter analyze individually for each package, feels outdated and unnecessary.

Reasoning

  • Performance Improvement: Running analysis once for the entire workspace is much faster than running it package by package.
  • Redundant Command: melos analyze no longer provides any significant benefit over Dart’s built-in commands, making it potentially unnecessary.

Additional context and comments

No response
@ricardodalarme ricardodalarme changed the title [v7.0.0-dev.2] request: [v7.0.0-dev.2] request: update melos analyze behaviour Jan 8, 2025
@spydon
Copy link
Collaborator

spydon commented Jan 8, 2025

True, I think it's time we just removed melos analyze.

@spydon spydon mentioned this issue Jan 8, 2025
Merged
7 tasks
@lishaduck
Copy link

We could do the same for format, and perhaps try to upstream what remains of list so that we could remove that too?

@spydon
Copy link
Collaborator

spydon commented Jan 9, 2025 •
edited
Loading

We could do the same for format, and perhaps try to upstream what remains of list so that we could remove that too?

The built-in format doesn't work directly with the packages right? It just works on a directory level.
I suggested that they should add a list command here: dart-lang/pub#4391 (comment)

@lishaduck
Copy link

The built-in format doesn't work directly with the packages right? It just works on a directory level.

Yeah, but is anything stopping anyone from running it in the root?

I suggested that they should add a list command here: dart-lang/pub#4391 (comment)

Don't they have one? (Not as sophisticated, but it's there)

@spydon
Copy link
Collaborator

spydon commented Jan 9, 2025

Yeah, but is anything stopping anyone from running it in the root?

Then it would format packages that are not included in the workspace too, which melos format avoids.

Don't they have one? (Not as sophisticated, but it's there)

Oh, perfect! I didn't know about that one.

@ricardodalarme
Copy link
Author

Yeah, I don't think we should drop the format command either. It only formats the packages you specify in the workspace. The dart format formats the entire directory you specify

spydon added a commit that referenced this issue Jan 9, 2025
@spydon
fix!: Remove analyze command (#829)
ab70ef3 
<!--
  Thanks for contributing!

Provide a description of your changes below and a general summary in the
title

Please look at the following checklist to ensure that your PR can be
accepted quickly:
-->

## Description

Since pub workspaces now can run analyze within all packages in the
workspace we can now remove our analyze proxy command.

Closes: #827 

## Type of Change

<!--- Put an `x` in all the boxes that apply: -->

- [ ] ✨ `feat` -- New feature (non-breaking change which adds
functionality)
- [x] 🛠️ `fix` -- Bug fix (non-breaking change which fixes an issue)
- [ ] ❌ `!` -- Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] 🧹 `refactor` -- Code refactor
- [ ] ✅ `ci` -- Build configuration change
- [ ] 📝 `docs` -- Documentation
- [ ] 🗑️ `chore` -- Chore
@spydon
Copy link
Collaborator

spydon commented Jan 10, 2025

Done in #829

@spydon spydon closed this as completed Jan 10, 2025
@spydon spydon mentioned this issue Jan 21, 2025
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix!: Remove analyze command invertase/melos
3 participants
@spydon @ricardodalarme @lishaduck