New feature: mbed releases #576
Conversation
…am or library. This enables listing of tags matched against common release naming patterns, e.g. v1.2, r1.3.5, mbed-os-5.1.2, etc The command supports the following switches -a|-all - Show all, including release candidates, alpha, betas -r|--recursive - Show releases for all libraries and sub-libraries as well Also `mbed ls` uses some of the routines from `mbed releases` and now shows associated tags/releases
Copy edit for consistent phrasing and active voice.
There was a problem hiding this comment.
This is really slick! This is going to be such a great help to our users, I know people have had issues with this in the past, great to see it being handled so well here!
I've added a few comments to the docs/logs to help clarify the finer points of the new features.
Awesome work!
mbed/mbed.py
Outdated
| repo = Repo.fromrepo() | ||
| tags = repo.scm.gettags() | ||
| revtags = repo.scm.gettags(repo.rev) if repo.rev else [] # associated tags with current commit | ||
| revstr = ('#'+repo.rev[:12]+(', tags:'+', '.join(revtags[0:2]) if len(revtags) else '')) if repo.rev else '' |
There was a problem hiding this comment.
This is a bit of a formatting nitpick, but I think it helps users more quickly scan for matching revisions.
Currently its setup to print like the following:
`- mbed-os (#182bbd51bc8d, tags:latest, mbed-os-5.6.5)
What do you think of the following changes?
- Add a space between
tags:and the list of tags - Wrap the list of tags with
[]to help differentiate the comma-separated list from the commit that comes before it
So as an example:
`- mbed-os (#182bbd51bc8d, tags: [latest, mbed-os-5.6.5])
There was a problem hiding this comment.
I've added the space after tags. The list wrapped in [] seems a lot like taken from the json/javascript world though.
There was a problem hiding this comment.
Thanks for adding the spaces. We don't have to use [], I was mostly thinking about how we can we show that latest and mbed-os-5.6.5 are part of the tags instead of another item in the existing list (the one containing #182bbd51bc8d and tags: ...). My eyes in particular have trouble separating all the different bits in this line since it contains a lot of (very useful) information.
If you think () would be a better container than [], that's totally fine with me! But if you don't think this is necessary, then no biggie!
README.md
Outdated
| @@ -741,6 +756,42 @@ The update command fails if there are changes in your program or library that `m | |||
|
|
|||
| ### Updating to an upstream version | |||
|
|
|||
| Before updating a program or a library, it's good to know the names of the stable releases, usually marked with a tag using a common format, such as `1.2`, `v1.0.1`, `r5.6`, `mbed-os-5.6` and so on. | |||
There was a problem hiding this comment.
This line is really helpful for determining what will be considered "stable" release. It looks like you're using a regular expression to determine this.
Would it also be possible to add to the docs the specific rules you're checking for? For instance, "Starts with v, r mbed-os-, etc" and "does not end in rcxxxx". Just a list detailing the specific conditions would be super helpful in addition to the example you gave above.
|
@screamerbg Great work, took my original ideas, improved them and rolled out a really, really useful command addition! |
mbed/mbed.py
Outdated
| for ref in refs: | ||
| m = re.match(r'^(.+)\s+(.+)$', ref) | ||
| if m and (not rev or m.group(1).startswith(rev)): | ||
| if re.match(r'refs\/tags\/', m.group(2)): # only tags |
There was a problem hiding this comment.
Why have a series of regexps? Could this all be folded into one regexp?
mbed/mbed.py
Outdated
| if re.match(r'refs\/tags\/', m.group(2)): # only tags | ||
| t = re.sub(r'refs\/tags\/', '', m.group(2)) | ||
| if re.match(r'^(.+)\^\{\}$', t): # detect tag "symlink" | ||
| t = re.sub(r'\^\{\}$', '', t) # remove "symlink" chars, e.g. some-tag^{} |
There was a problem hiding this comment.
Could you explain what a "tag symlink" is? I don't see any in my git show-ref output.
There was a problem hiding this comment.
try git show-ref --tags --dereference. A quote from stackoverflow:
In git, a "normal" (annotated, not lightweight) tag is an object unto itself, containing metadata and the SHA1 of the object it tags. There's a pretty picture in the section of the git community book on the git object model (scroll to the bottom). So, when you use show-ref on a normal tag, it will normally give you information about the tag object. With the -d/--dereference option, it will dereference the tag into the object the tag refers to, and provide information about it instead. And a note on lightweight vs. annotated tags, in case you aren't aware of that: a lightweight tag is created by using git tag (i.e. without any of the metadata-providing options like -a, -s, or -u). It's not a tag object at all, just a ref pointing straight to the object you've tagged. If you provide one of those options, you're attaching metadata to the tag, so git creates a tag object to hold that.
There was a problem hiding this comment.
@screamerbg That tells me that some tags are references and some are tag objects. How do "symlinks" get involved here?
mbed/mbed.py
Outdated
| @@ -2123,21 +2163,63 @@ def sync(recursive=True, keep_refs=False, top=True): | |||
| "View the dependency tree of the current program or library.")) | |||
| def list_(detailed=False, prefix='', p_path=None, ignore=False): | |||
| repo = Repo.fromrepo() | |||
| revtags = repo.scm.gettags(repo.rev) if repo.rev else [] | |||
| revstr = ('#'+repo.rev[:12]+(', tags: '+', '.join(revtags[0:2]) if len(revtags) else '')) if repo.rev else '' | |||
There was a problem hiding this comment.
Could you add spaces around the + operators in this line?
revstr = ('#' + repo.rev[:12] + (', tags: ' + ', '.join(revtags[0:2]) if len(revtags) else '')) if repo.rev else
mbed/mbed.py
Outdated
|
|
||
| print "%s (%s)" % (prefix + (relpath(p_path, repo.path) if p_path else repo.name), ((repo.url+('#'+str(repo.rev)[:12] if repo.rev else '') if detailed else str(repo.rev)[:12]) or 'no revision')) | ||
| print "%s (%s)" % (prefix + (relpath(p_path, repo.path) if p_path else repo.name), ((repo.url+('#'+str(repo.rev)[:12] if repo.rev else '') if detailed else revstr) or 'no revision')) |
There was a problem hiding this comment.
Could you add a single space around the + operators in this line too?
print "%s (%s)" % (prefix + (relpath(p_path, repo.path) if p_path else repo.name), ((repo.url + ('#' + str(repo.rev)[:12] if repo.rev else '') if detailed else revstr) or 'no revision'))
mbed/mbed.py
Outdated
| repo = Repo.fromrepo() | ||
| tags = repo.scm.gettags() | ||
| revtags = repo.scm.gettags(repo.rev) if repo.rev else [] # associated tags with current commit | ||
| revstr = ('#'+repo.rev[:12]+(', tags: '+', '.join(revtags[0:2]) if len(revtags) else '')) if repo.rev else '' |
There was a problem hiding this comment.
Could you add a space around the + operators here too?
revstr = ('#' + repo.rev[:12] + (', tags: ' + ', '.join(revtags[0:2]) if len(revtags) else '')) if repo.rev else ''
mbed/mbed.py
Outdated
| rels = [] | ||
| for tag in tags: | ||
| if re.match(regex_rels, tag[1]): | ||
| rels.append(tag[1] + " %s%s" % ('#'+tag[0] if detailed else "", " <- current" if tag[1] in revtags else "")) |
There was a problem hiding this comment.
Another + operator in need of some spaces.
rels.append(tag[1] + " %s%s" % ('#' + tag[0] if detailed else "", " <- current" if tag[1] in revtags else ""))
mbed/mbed.py
Outdated
| rels.append(tag[1] + " %s%s" % ('#'+tag[0] if detailed else "", " <- current" if tag[1] in revtags else "")) | ||
|
|
||
| # Print header | ||
| print "%s (%s)" % (prefix + (relpath(p_path, repo.path) if p_path else repo.name), ((repo.url+('#'+str(repo.rev)[:12] if repo.rev else '') if detailed else revstr) or 'no revision')) |
There was a problem hiding this comment.
Spaces around the + op here too.
print "%s (%s)" % (prefix + (relpath(p_path, repo.path) if p_path else repo.name), ((repo.url + ('#'+str(repo.rev)[:12] if repo.rev else '') if detailed else revstr) or 'no revision'))
mbed/mbed.py
Outdated
| rprefix += '| ' if recursive and len(repo.libs) > 1 else ' ' | ||
| if len(rels): | ||
| for rel in rels: | ||
| print rprefix+'* '+rel |
There was a problem hiding this comment.
These + ops could use a space too.
print rprefix + '* ' + rel
mbed/mbed.py
Outdated
| for rel in rels: | ||
| print rprefix+'* '+rel | ||
| else: | ||
| print rprefix+'No release tags detected' |
There was a problem hiding this comment.
This + operator could use some separating spaces as well.
print rprefix + 'No release tags detected'
screamerbg
force-pushed
the
feature_releases
branch
from
783260e to
ab880e6
Compare
screamerbg
force-pushed
the
feature_releases
branch
from
ab880e6 to
e08f184
Compare
|
Thanks everyone. Let's get this in. |
This feature aims to enhance the workflow for the majority of developers who are looking for stable releases of a program or a library, where these stable revisions usually marked with a source control (Git/Hg) tag.
Workflow
mbed import <url>to import a program. Alternatively usembed new <path>to create a programmbed lsto list the structure of their program.mbed releases [-r]to list available releases for a program or a library <- this PR is aiming at improving thismbed update <release tag>to update to the releasePreviously a developer had to fiddle with the relevant source control management (SCM) tool for their program or library. This is somewhat inconvenient as the command set between Git and Mercurial are very different (
hg tagsvsgit show-ref --tags --dereference). Additionally the returned tags might be randomly formatted, which in a repository with large set of tags makes it hard to spot which are release/stable tags.mbed releasesfilters the displayed release tags based on commonly tag format (see https://git-scm.com/book/en/v2/Git-Basics-Tagging)How this works
Listing the releases for the current program or library is done via
mbed releases, e.g.Command switch
-ais used to list release-hash pairs, e.g.While by default this command works for the current program or library, the
-rswitch enables recursive listing in a tree format similar tombed ls, e.g.Also, one can list unstable release tags, usually release candidates, alphas, betas, using the
-uswitch. This relaxes the release/tag name matcher to include a wider set of tags (see https://git-scm.com/book/en/v2/Git-Basics-Tagging again)Lastly, the implementation enhances
mbed lsas well and makes it release/tag "aware". Here's an example:Documentation
Documentation is included with this PR. Firstly
mbed releasesis briefly mentioned in the Importing/Creating section. Then it's more reviewed in detail in the "Update" section as a prerequisite for an update workflow (as per the description above).@AnotherButler please review
Tests
Releases are listed recursively (using
mbed releases -r) for both Git, Mercurial and Mbed 2/Classic repositories as part of the circle CI tests.The feature was inspired by a conversation with @adbridge regarding releases process, release candidates, release tags, release XYZ, and end-developer workflows :)
CC @sg- @adbridge @MarceloSalazar