-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
feat: verify --from
and --to
parameters to be valid git pointer
#248
feat: verify --from
and --to
parameters to be valid git pointer
#248
Conversation
Codecov Report
@@ Coverage Diff @@
## main #248 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 614 621 +7
=========================================
+ Hits 614 621 +7
Continue to review full report at Codecov.
|
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.
Nice contribution, thanks
I have a few remarks
and also it requires unit test for the case where to and from are a git sha and not a git sha
I can help on that, just give access to the repo if you want me to write the tests
Thanks for this contribution @denisab85 ! We currently do not have this kind of usage issue reported but I consider it a nice way to tell the user what is wrong early. |
A colleague of mine faced this issue when trying to use a shortened sha. |
--from
and --to
parameters to be valid git pointer
@denisab85 we have merged a big PR with a new way to deal with IO. |
Thanks for letting me know. I merged in from upstream. |
Hi @denisab85 Could you give me access to your fork ? |
Hi, Sebastien.
Try now please.
Sorry, it’s been really difficult to concentrate on work after the war in
Ukraine burst out. I’ve been on vacation and away from coding.
On Wed, 9 Mar 2022 at 06:58, Sebastien ***@***.***> wrote:
Hi @denisab85 <https://github.com/denisab85>
Could you give me access to your fork ?
I would like to contribute
—
Reply to this email directly, view it on GitHub
<#248 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AD7AHBF7M6NJT2ASX64WZGDU7CN6XANCNFSM5PCWG2UQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Best regards,
Denis Abakumov.
|
Hi @denisab85 ! Thanks for giving me access Take care buddy |
bb59739
to
8cad00d
Compare
@mehdisfdc this one will require in depth smoke testing. |
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.
LGTM
ff19862
to
880a5ca
Compare
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.
LGTM
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.
LGTM
880a5ca
to
fa3dd68
Compare
Code Climate has analyzed commit fa3dd68 and detected 0 issues on this pull request. View more on Code Climate. |
What does this pull request contains
The 'from' and 'to' parameters will be verified to be valid git commits
Explain your changes
Does this close any currently open issues?
closes #
Any particular element to being able to test locally
Any other comments?
Where has this been tested?
Operating System: …
yarn version: …
node version: …
git version: …
sfdx version: …
sgd plugin version: …