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

refactor(all): Remove all manual dependency_overrides #1628

Merged

Conversation

spydon
Copy link
Contributor

@spydon spydon commented Mar 15, 2023

Description

Manual dependency_overrides in the examples shouldn't be needed since Melos already should handle those.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I titled the PR using Conventional Commits.
  • I did not modify the CHANGELOG.md nor the pubspec.yaml files.
  • All existing and new tests are passing.
  • The analyzer (flutter analyze) does not report any problems on my PR.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • No, this is not a breaking change.

@spydon spydon mentioned this pull request Mar 15, 2023
6 tasks
@spydon spydon changed the title refactor: Remove all manual dependency_overrides refactor(all): Remove all manual dependency_overrides Mar 15, 2023
@vbuberen
Copy link
Collaborator

Unfortunately it doesn't solve the problem I mentioned in #1619 (comment) because it is not about using overrides, but because melos version bumps version of share_plus_platform_interface and during melos publish dry run can't resolve the bumped version of platform interface for share_plus.
There were a few mentions that the problem is about some changes in pub and it is not Melos related.

Screenshot 2023-03-16 at 12 03 04

I have tried your PR with additional similar change in share_plus you did, but haven't seen any changes in behavior during melos bootstraping, etc.
Additionally I would prefer to have dependencies from path in example apps as really use Melos only when need to do release, not during work on plugins.

@spydon
Copy link
Contributor Author

spydon commented Mar 16, 2023

Makes sense that it didn't solve anything then.

Additionally I would prefer to have dependencies from path in example apps as really use Melos only when need to do release, not during work on plugins.

You still have to run Melos bootstrap to work locally though, and then it would set up the examples to be dependent on the other packages in the repository?

@vbuberen
Copy link
Collaborator

You still have to run Melos bootstrap to work locally though, and then it would set up the examples to be dependent on the other packages in the repository?

No, I don't. I started to use Melos only recently and only for purpose of publishing. Before that just opened a project of a plugin in question and work on it.

@spydon
Copy link
Contributor Author

spydon commented Mar 17, 2023

No, I don't. I started to use Melos only recently and only for purpose of publishing. Before that just opened a project of a plugin in question and work on it.

Interesting, how do you work with changing packages that are dependent on other packages in the repository, like the interfaces for example? Because if you haven't run melos bootstrap it would pull the interface package from pub instead of using the local one?

@vbuberen
Copy link
Collaborator

Interesting, how do you work with changing packages that are dependent on other packages in the repository, like the interfaces for example?

Probably, I haven't bumped into this case so far 🤷🏻

@spydon
Copy link
Contributor Author

spydon commented Apr 4, 2023

@vbuberen should I close this or do you want to start using melos all the way? :)

@vbuberen
Copy link
Collaborator

vbuberen commented Apr 5, 2023

Well, let's merge it if Melos takes care of this stuff during versioning, but don't really see much benefits.

@vbuberen vbuberen enabled auto-merge (squash) April 5, 2023 09:46
auto-merge was automatically disabled April 5, 2023 15:13

Head branch was pushed to by a user without write access

@spydon spydon force-pushed the spydon/remove-dependency-overrides branch from 9ac1213 to 0a850fc Compare April 5, 2023 15:13
@spydon spydon force-pushed the spydon/remove-dependency-overrides branch from 0a850fc to 23a1b70 Compare April 5, 2023 15:13
@spydon
Copy link
Contributor Author

spydon commented Apr 5, 2023

@vbuberen something strange had happened when this branch was updated with main, but it's fixed now. The auto-complete was cancelled though.

@vbuberen
Copy link
Collaborator

vbuberen commented Apr 5, 2023

Yeah, had to manually restart some of our flaky android integration tests, but now everything seems fine.

@vbuberen vbuberen merged commit f7673fb into fluttercommunity:main Apr 5, 2023
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.

2 participants