Skip to content

feat: Add short sha to VersionContext Result#82

Merged
Kieranties merged 2 commits into
Kieranties:masterfrom
asci13:feature/ShortSha
Jun 15, 2019
Merged

feat: Add short sha to VersionContext Result#82
Kieranties merged 2 commits into
Kieranties:masterfrom
asci13:feature/ShortSha

Conversation

@asci13

@asci13 asci13 commented Jun 14, 2019

Copy link
Copy Markdown
Contributor

Description

Added Sha7 (instead of ShortSha - afaik thats one of the official names of it) to the Result object.
Let me know if you'd prefer ShortSha instead.

Fixes #76

Change Type

  • Documentation: Changes to docs are included
  • Infrastructure: Changes to build scripts/non-deliverables
  • Bug Fix: Fixes an issue in implementation
  • New Feature: Adds new functionality in a non-breaking manner
  • Breaking Change: Implements a fix or feature in a breaking manner

Quality

  • Unit tests have been added/updated
  • Local builds/testing are successful
  • Code coverage has not decreased
  • Code comments have been provided (where appropriate)
  • Core styles have been followed
  • Documentation has been added/updated

@asci13 asci13 requested a review from Kieranties as a code owner June 14, 2019 09:38
@asci13 asci13 changed the title #76 Add short sha to VersionContext Result. feat: Add short sha to VersionContext Result Jun 14, 2019
@asci13

asci13 commented Jun 14, 2019

Copy link
Copy Markdown
Contributor Author

@Kieranties as promised :) Please take a look (especially the property naming) and let me know what to change.

@Kieranties

Copy link
Copy Markdown
Owner

@asci13 Seems to be a 'quirk' with Azure Devops that the PR does not have access to to the artifacts to download simpleversion. Seems the build is ran as you as the user, and you don;t have permissions to the internal feed. I assumed the build ran as the build service user. Will address this shortly.

@Kieranties Kieranties left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@asci13 Could you add some info to https://github.com/Kieranties/SimpleVersion/blob/master/docs/articles/results.md ?

One minor thing to check out, otherwise looks good! Should be able to complete pending docs.

Comment thread src/SimpleVersion.Core/Pipeline/VersionContext.cs Outdated
@Kieranties

Copy link
Copy Markdown
Owner

@asci13 I've updated the build to pull simpleversion from nuget for builds. However, Azure devops doesn't appear to be using the change for the PR. You'll need to rebase your work on master and update the PR :-(

@asci13 asci13 force-pushed the feature/ShortSha branch from a1b85d3 to c1aeb80 Compare June 15, 2019 07:55
@asci13

asci13 commented Jun 15, 2019

Copy link
Copy Markdown
Contributor Author

@Kieranties all done now i hope :)

@Kieranties Kieranties left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@asci13 Thanks! I'm still not certain about the usage of sha7 vs shortsha and can't find consistent usage of either term in other git solutions. I'll take this as is now, but may address the language before a v0.2 release.

@Kieranties Kieranties merged commit 962e9d7 into Kieranties:master Jun 15, 2019
@Kieranties

Copy link
Copy Markdown
Owner

@asci13 Shipped. You'll find the assets available on nuget.org soon. In the mean time: https://github.com/Kieranties/SimpleVersion/releases/tag/v0.2.0-alpha1.25

@asci13

asci13 commented Jun 17, 2019

Copy link
Copy Markdown
Contributor Author

@Kieranties no stress with the artifacts, just wanted to contribute :)

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.

Return short sha value in the result object

2 participants