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

feat: add support for melos_overrides.yaml + command/bootstrap/dependencyOverridePaths #410

Merged
merged 11 commits into from
Dec 20, 2022
Merged

feat: add support for melos_overrides.yaml + command/bootstrap/dependencyOverridePaths #410

merged 11 commits into from
Dec 20, 2022

Conversation

hacker1024
Copy link
Contributor

Description

This adds two related features related to the feature request #409.

  1. Adds a melos_overrides.yaml file, that can be used to override any field in melos.yaml (for making local, untracked changes)

    Example use cases: local dependency_overrides, customized IDE project generation

  2. Adds a dependency_overrides configuration option, to add non-workspace packages to each package's dependency_overrides.yaml.

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

@CLAassistant
Copy link

CLAassistant commented Oct 18, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@hacker1024 hacker1024 changed the title Feature/dependency overrides melos_overrides.yaml + a new dependency_overrides option Oct 18, 2022
@hacker1024 hacker1024 changed the title melos_overrides.yaml + a new dependency_overrides option feat: melos_overrides.yaml + a new dependency_overrides option Oct 18, 2022
@hacker1024 hacker1024 marked this pull request as draft October 18, 2022 11:47
@hacker1024 hacker1024 marked this pull request as ready for review October 18, 2022 12:21
@hacker1024 hacker1024 force-pushed the feature/dependency_overrides branch from d629e14 to 71ee869 Compare October 18, 2022 12:22
blaugold
blaugold reviewed Oct 21, 2022
View reviewed changes
Copy link
Collaborator

@blaugold blaugold left a comment

Choose a reason for hiding this comment

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

Thank you for contributing!

packages/melos/lib/src/workspace_configs.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/workspace_configs.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/workspace_configs.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/workspace_configs.dart Outdated Show resolved Hide resolved
docs/configuration/overview.mdx Outdated Show resolved Hide resolved
packages/melos/lib/src/common/utils.dart Outdated Show resolved Hide resolved
packages/melos/lib/src/commands/bootstrap.dart Outdated Show resolved Hide resolved
@hacker1024 hacker1024 requested a review from blaugold October 26, 2022 23:04
@blaugold
Copy link
Collaborator

The changes in _generateTemporaryProjects were not quite right yet, but I've fixed that.

The PR LGTM as it is right now, but I got thinking whether it would be better if the dependencyOverrides in melos.yaml worked like those in a pubspec.yaml. Currently, you can only use path dependencies for workspace-wide overrides, but sometimes it's useful to use different sources (pub.dev, git). WDYT @hacker1024?

@hacker1024
Copy link
Contributor Author

hacker1024 commented Oct 30, 2022 •
edited
Loading

For my specific usecase, linking to local projects that weren't part of the workspace, I wasn't concerned about supporting packages from other sources.

Allowing them would be nice, but it adds complexity. Relative path packages would need to be adjusted depending on the directory of each workspace package, and that goes for git packages with local URIs, too. Furthermore, the glob functionality would be unavailable.

The one change that I think could be useful would be to allow packages from directories outside the workspace directory, as is supported in path dependencies, but that's also hard to implement due to the glob resolution design, and can be worked around with an untracked symlink anyway.

@hacker1024
Copy link
Contributor Author

@blaugold gentle bump, is there anything else that needs to be done?

Salakar
Salakar reviewed Dec 14, 2022
View reviewed changes
docs/configuration/overview.mdx Outdated
@@ -88,6 +91,22 @@ ignore:
> You can also expand the scope of ignored packages on a per-command basis via
> the [`--scope` filter](/filters#scope) flag.

## `dependencyOverrides`
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we move this to command/bootstrap/dependencyOverrides since this is specific to the bootstrap command?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good point!

hacker1024 and others added 11 commits December 14, 2022 18:22
@hacker1024 @blaugold
feat: allow overriding melos.yaml fields with melos_overrides.yaml
67effa7
@hacker1024 @blaugold
feat: add configuration section for manual dependency overrides (inve…
27eccf5 
…rtase#409)

This is limited to packages within the workspace directory, but symlinks can be used to work around this.

invertase#43
@hacker1024 @blaugold
docs: document the dependency_overrides field
82e65d6
@hacker1024 @blaugold
docs: document the melos_overrides.yaml file
2e6969b
@hacker1024 @blaugold
fix: deep-merge overridden YAML values
c095c27
@hacker1024 @blaugold
fix: make loaded config YAML maps mutable to allow merging overrides
3692913
@hacker1024 @blaugold
feat: rename dependency_overrides option to dependencyOverrides
b127754
@hacker1024 @blaugold
refactor: rename unYaml utility function to toPlainObject
107d373
@hacker1024 @blaugold
fix: prioritize dependency overrides in pubspec.yaml over Melos depen…
e9175f8 
…dencyOverrides
@hacker1024 @blaugold
feat: use dependencyOverrides in the original bootstrap mechanism
5c8d47c
@blaugold
Move dependencyOverridePaths to command/bootstrap
da2c275
@blaugold blaugold force-pushed the feature/dependency_overrides branch from 93ad616 to da2c275 Compare December 20, 2022 18:40
@blaugold
Copy link
Collaborator

Sorry, it took me so long to get to this PR.

I've renamed dependencyOverrides to dependencyOverridePaths so we can use dependencyOverrides for an option that works like dependency_overrides in pubspec.yaml in the future if we want to. I've also moved the option to command/bootstrap.

LGTM now, and thanks for the contribution @hacker1024.

@blaugold blaugold changed the title feat: melos_overrides.yaml + a new dependency_overrides option feat: add support for melos_overrides.yaml + command/bootstrap/dependencyOverridePaths Dec 20, 2022
@blaugold blaugold linked an issue Dec 20, 2022 that may be closed by this pull request
request: Untracked, local additions to pubspec_overrides.yaml #409
Closed
1 task
@blaugold blaugold merged commit bf26b52 into invertase:main Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Salakar Salakar Salakar left review comments

@blaugold blaugold blaugold approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

request: Untracked, local additions to pubspec_overrides.yaml
4 participants
@hacker1024 @CLAassistant @blaugold @Salakar