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

Adding Melos for monorepo management #681

Closed
wants to merge 13 commits into from
Closed

Conversation

jmatth
Copy link
Contributor

@jmatth jmatth commented Jul 7, 2022

See #680 for details. Let me know if the update to the root readme should be moved or copied to any of the individual project readmes. I was on the fence about doing so but it seems potentially annoying to maintain setup instructions in each individual package as the repo grows. Maybe just add a line to each one saying to refer to the root readme to set up your development environment?

Closes #680.

# Override to local mono-repo path so devs can test this repo
# against changes that they're making to other mono-repo packages
attributed_text:
path: ../../attributed_text
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason that this one dependency is left under overrides, but all the others are listed above as regular deps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This dependency is transitive, the others are direct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, can you adjust the comment to make that clear? The other dependencies are also "to local mono-repo path" so it's not obvious why this one is different. But if you explain that it's transitive then the reader should understand.

Copy link
Contributor Author

@jmatth jmatth Jul 8, 2022

Choose a reason for hiding this comment

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

Done, let me know if you want me to revise the new comments to be more verbose

@matthew-carroll
Copy link
Contributor

My plan is to get this PR and the AttributedText PR merged after the next couple releases, because we're almost done with a couple deliverables.

BTW, it looks like Windows tests are failing in CI

@jmatth
Copy link
Contributor Author

jmatth commented Jul 18, 2022

My plan is to get this PR and the AttributedText PR merged after the next couple releases, because we're almost done with a couple deliverables.

Cool, I'll keep the branches up to date with main in the meantime.

BTW, it looks like Windows tests are failing in CI

Well this is annoying. Turns out there was a bug introduced in Dart 2.17.0 that causes concurrent pub get processes to fail (dart-lang/pub#3420). They merged a fix back in May but it still hasn't reached the Dart SDK stable channel. I'll investigate a workaround for now.

@jmatth
Copy link
Contributor Author

jmatth commented Nov 22, 2022

@matthew-carroll This is ready for another look if you're still interested in this change. #680 describes the reasoning and proposes an alternative if you don't want to add a dependency on an external tool.

@matthew-carroll
Copy link
Contributor

Yeah, I'd like to get this in, but can we start with the attribution range change?

@matthew-carroll
Copy link
Contributor

I'm going to close this because it's so out of date. We still want this. I'll probably refactor for Melos before the next major release. That will be a different PR.

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.

Add Melos for monorepo management
2 participants