-
Notifications
You must be signed in to change notification settings - Fork 118
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
compare: add --decorate #662
compare: add --decorate #662
Conversation
When a project is not on the `manifest-rev` it will often be on a branch or tag. Use the `%d` format to make `west compare` show that information. As discussed in zephyrproject-rtos#643 and before, git status cannot be fully trusted to display branch information. Sample new output: ``` west compare === zephyr (zephyr): --- manifest-rev: e40859f78712 (some_tag) Revert "dma: dw: Do ... HEAD: e803b77463b4 (zephyrproject/main) samples: net: ... --- status: HEAD detached at zephyrproject/main nothing to commit, working tree clean ``` Signed-off-by: Marc Herbert <marc.herbert@intel.com>
['log', '-1', '--color=never', '--pretty=%h %s', rev], | ||
title = project.git( | ||
['log', '-1', '--color=never', '--pretty=%h%d %s', | ||
'--decorate-refs-exclude=refs/heads/manifest-rev', |
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.
Did you check that this doesn't introduce a requirement for a more recent version of git?
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.
No I hadn't, my bad. These options were introduced in git commit v2.16.0-rc0~64^2 git/git@65516f586b69307f977
Git 2.16.0 was released in January 2018, 5+ years ago.
I couldn't find what is the minimum git version required by west, where is it defined?
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.
That's fine then; we have a if self.git_version_info < (2, 22):
check up on line 659.
(LGTM otherwise) |
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.
@marc-hb this LGTM now but offline you mentioned this being broken by west only fetching by URL -- can you confirm or deny that this is OK to merge?
It's not "broken", it may make it slightly less useful, showing fewer decorations in some cases. I still find this PR of mine very useful ;-) |
When a project is not on the
manifest-rev
it will often be on a branch or tag. Use the%d
format to makewest compare
show that information.As discussed in #643 and before, git status cannot be fully trusted to display branch information.
Sample new output: