-
Notifications
You must be signed in to change notification settings - Fork 230
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
Add 'tag_pattern' feature to git dependencies #4427
base: master
Are you sure you want to change the base?
Conversation
case final String descriptionTagPattern: | ||
tagPattern = descriptionTagPattern; | ||
case null: | ||
tagPattern = 'v*'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering, if we should support multiple patterns, and if we should then default to two:
tagPatterns = ['v*', '$package-v*'];
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm - considering this again I think we should actually not default to anything. I think it is probably better to be unopinionated here
lib/src/source/git.dart
Outdated
'tag_pattern': description.tagPattern, | ||
'resolved-ref': resolvedRef, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use tag-pattern
for consistency? (it would be inconsistent with other fields in pubspec.yaml
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm - this is pubspec.lock
.
In pubspec.yaml we use _
(at least in pubspec_overrides
and dev_dependencies
.
So when we copy the field from the pubspec.yaml to pubspec.lock it is confusing if we change the spelling.
At the same time it is confusing that we have resolved-ref
in the pubspec.lock.
Can we find a single-word alternative?
@@ -72,6 +75,32 @@ class GitSource extends CachedSource { | |||
'string.'); | |||
} | |||
path = descriptionPath; | |||
|
|||
// TODO: can we avoid relying on key presence? | |||
if (description.containsKey('tag_pattern')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about simply using tags
and then support one or more patterns?
dependencies:
foo:
git:
url: https://foo.bar.git/repo
tags: v*
and
dependencies:
foo:
git:
url: https://foo.bar.git/repo
tags:
- v*
- foo-v*
or maybe not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - that might work
Re: #1250
With this proposal you can use a git repo as a versioned package repository.
For example write:
And all commits in the git repo that are tagged according to
v*
will be inspected. If they contain a pubspec.yaml withname: foo
they will be considered for resolution.The
tag_pattern
syntax is the one used bygit tag --list
. See https://git-scm.com/docs/git-tag#Documentation/git-tag.txt---listThis feature is explicitly opt-in:
git
dependency without atag_pattern
retain the existing behavior.Open questions: