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

feat!: Migrate to use the Pub workspaces feature #816

Merged
merged 8 commits into from
Jan 7, 2025

Conversation

spydon
Copy link
Collaborator

@spydon spydon commented Jan 6, 2025

Description

This PR migrates so that we rely on the pub workspaces feature instead of creating the pubspec_overrides.yaml file.

Docs update and migration instructions are soon coming up.

Closes: #747

Type of Change

  • feat -- New feature (non-breaking change which adds functionality)
  • 🛠️ 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

Copy link

docs-page bot commented Jan 6, 2025

To view this pull requests documentation preview, visit the following URL:

docs.page/invertase/melos~816

Documentation is deployed and generated using docs.page.

@spydon spydon requested a review from exaby73 January 6, 2025 18:01
@lishaduck lishaduck mentioned this pull request Jan 6, 2025
1 task
@spydon spydon mentioned this pull request Jan 6, 2025
1 task
@spydon spydon merged commit 3602d90 into main Jan 7, 2025
10 checks passed
@spydon spydon deleted the feat/migrate-to-workspaces branch January 7, 2025 10:29
Comment on lines +27 to +32
After the [Pub Workspaces feature](https://dart.dev/tools/pub/workspaces) was
introduced in Dart 3.6.0, it is no longer strictly necessary to run `melos
bootstrap`, since all the packages are already linked together. However, there
are still some benefits to running `melos bootstrap`, because you need to run
`pub get` in each package to initialize the workspace, and `melos bootstrap`
will do that for you.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because you need to run pub get in each package to initialize the workspace

@spydon I've just been running dart pub get in any folder (in a non-Melos workspace) and it initializes the whole workspace. That makes sense to me because the packages have a shared resolution.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're correct!
I read the instructions here sloppily: https://dart.dev/tools/pub/workspaces

You need to run dart pub get once for each package.

But that section is of course about if you aren't using the pub workspaces feature.

Copy link

@Levi-Lesches Levi-Lesches Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might not a matter of interpretation but this paragraphs reads to me as:

Once you start using Pub Workspaces, you don't need to run melos bootstrap, but it's still convenient since it will run dart pub get in each sub-package for you

(It does say "each package in the _workspace" too!)

It would probably help to re-word this paragraph to specifically discuss how Melos and melos bootstrap play with the Workspaces feature. In general I had a hard time finding a list of what Melos can do that workspaces cannot (eg, specify that Melos can publish all sub-packages while pub cannot, but other pub commands do work recursively)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed it up here: #822

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed you second comment!
Yeah, I guess that we should clarify that on the intro page of the docs, is it something you'd like to PR? :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I don't actually know the details of melos -- that's why I came here, was hoping to find such a list!

spydon added a commit that referenced this pull request Jan 7, 2025
<!--
  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

We no longer need to run `pub get` in all packages, in one package, or
the workspace root is enough.

Fixes the issue mentioned here:
#816 (comment)

## 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
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.

request: pub workspaces
2 participants