Skip to content
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

Feature/sg optional progressbar prediction #1465

Conversation

EduardoPach
Copy link

What does this PR do:

Adds functionality mentioned in #1329

@EduardoPach
Copy link
Author

@Louis-Dupont can you review it?

@BloodAxe
Copy link
Contributor

Thanks for your PR. Unfortunately it look like it contains unsigned commits with is a hard requirement. Please check https://github.com/Deci-AI/super-gradients/blob/master/CONTRIBUTING.md for information on setting up the signed commits.

@EduardoPach
Copy link
Author

Thanks for your PR. Unfortunately it look like it contains unsigned commits with is a hard requirement. Please check https://github.com/Deci-AI/super-gradients/blob/master/CONTRIBUTING.md for information on setting up the signed commits.

Oh, I didn't notice this before, sorry. I actually set it up the gpg key, but I've used the wrong e-mail adresse. Do you know if there's an easy way to verify the commits I did or should I open a new PR with the verified commits?

@BloodAxe
Copy link
Contributor

No. What you need to do is create another branch starting from master, then move your commits from PR branch to the
new branch and force push it to the remote under the old name. Let's say your PR branch named 'feature/my_awesome_pr'
then you need to do the following:

git checkout master
git checkout -b feature/my_awesome_pr_signed
git merge --squash --no-commit feature/my_awesome_pr
git commit -S -m "My signed commit"
git push -f origin feature/my_awesome_pr_signed:feature/my_awesome_pr

Last command will overwrite your PR branch with the new signed commit containing all changes from the PR.

LMK if this snippet would work (written from memory). If yes, we will update contribution guide to contain this text to help other contributors (Btw, if it works feel free to make a second PR with these instructions).

@EduardoPach
Copy link
Author

No. What you need to do is create another branch starting from master, then move your commits from PR branch to the new branch and force push it to the remote under the old name. Let's say your PR branch named 'feature/my_awesome_pr' then you need to do the following:

git checkout master
git checkout -b feature/my_awesome_pr_signed
git merge --squash --no-commit feature/my_awesome_pr
git commit -S -m "My signed commit"
git push -f origin feature/my_awesome_pr_signed:feature/my_awesome_pr

Last command will overwrite your PR branch with the new signed commit containing all changes from the PR.

LMK if this snippet would work (written from memory). If yes, we will update contribution guide to contain this text to help other contributors (Btw, if it works feel free to make a second PR with these instructions).

It seemed to work! I'll update the CONTRIBUTING.md shortly

@BloodAxe
Copy link
Contributor

Glad it helped. Regarding PR to contributing md - probably no need, there is already one made by me #1486 :)

Copy link
Contributor

@BloodAxe BloodAxe left a comment

Choose a reason for hiding this comment

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

@EduardoPach
Copy link
Author

#1465 (comment)

Sorry for the delay, gonna fix this right now

@EduardoPach EduardoPach requested a review from BloodAxe October 3, 2023 14:31
@BloodAxe
Copy link
Contributor

BloodAxe commented Oct 5, 2023

@EduardoPach there is still some leftovers remaining. I left some comments for you to address.

@EduardoPach
Copy link
Author

@BloodAxe Now I believe should be done!

BloodAxe
BloodAxe previously approved these changes Oct 11, 2023
Copy link
Contributor

@BloodAxe BloodAxe left a comment

Choose a reason for hiding this comment

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

LGTM

@BloodAxe
Copy link
Contributor

@EduardoPach I think we are ready to merge your PR, however i still see some non-signed commits present in this PR, like this one.
Can you please fix this and we can finally merge it )

@EduardoPach EduardoPach force-pushed the feature/SG-optional-progressbar-prediction branch from 5b4fd8a to a6d4630 Compare October 18, 2023 00:53
@EduardoPach
Copy link
Author

@EduardoPach I think we are ready to merge your PR, however i still see some non-signed commits present in this PR, like this one. Can you please fix this and we can finally merge it )

Fixed!

@EduardoPach EduardoPach requested a review from BloodAxe October 23, 2023 21:43
@BloodAxe
Copy link
Contributor

BloodAxe commented Jan 8, 2024

Closing as stale

@BloodAxe BloodAxe closed this Jan 8, 2024
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.

2 participants