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

Devel ergo cub sn001 #681

Merged
merged 9 commits into from
Sep 26, 2024
Merged

Conversation

pattacini
Copy link
Member

Update upstream ergoCubSN001 conf files.

Copy link
Member Author

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

Just a few inline comments before merging.

ergoCubSN001/calibrators/left_arm-calib.xml Show resolved Hide resolved
ergoCubSN001/ergocub.xml Outdated Show resolved Hide resolved
@S-Dafarra
Copy link
Contributor

S-Dafarra commented Sep 26, 2024

There are two commits that are workaround for hardware issues we have currently in the robot. Namely:

Corresponding tickets:

Let me know if you want to proceed anyhow. If you do, can we avoid to squash? In this way it gets easy to revert them when the robot is fixed

@pattacini
Copy link
Member Author

Let me know if you want to proceed anyhow. If you do, can we avoid to squash? In this way it gets easy to revert them when the robot is fixed

We need to avoid squashing anyhow for the reason reported in par. 5.i at https://github.com/robotology/robots-configuration/blob/master/.github/CONTRIBUTING.md#-fast-pace-local-workflow.

I may try to cherry-pick the other commits in upstream@devel and check whether this PR will get updated with the remaining commits. Once verified, I can close the PR unmerged.

Does it sound ok?

@pattacini
Copy link
Member Author

pattacini commented Sep 26, 2024

I may try to cherry-pick the other commits in upstream@devel and check whether this PR will get updated with the remaining commits. Once verified, I can close the PR unmerged.

Or even quicker for me, I could merge the PR and then revert those two commits.

What do you prefer?

@S-Dafarra
Copy link
Contributor

I have booked the robot at 13. I can rebase the branch on upstream@devel, and back up those two commits in another branch. Then, we can merge this (without the two commits) and I will take care of bringing those commits back into devel-ergoCubSN001. Would that work?

@pattacini
Copy link
Member Author

Perfect! Go ahead this way.

@icub-tech-iit-bot icub-tech-iit-bot force-pushed the devel-ergoCubSN001 branch 2 times, most recently from 61e55ce to a51affc Compare September 26, 2024 11:36
Copy link
Contributor

@S-Dafarra S-Dafarra left a comment

Choose a reason for hiding this comment

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

Done @pattacini, the PR is ready to go

@pattacini
Copy link
Member Author

Thanks!
I'll merge it after the CI.

@S-Dafarra
Copy link
Contributor

One thing that I noticed is that the bot has push-force permission. I think it might be better to protect the branch from push force and allow only a handful of people doing it

@pattacini
Copy link
Member Author

pattacini commented Sep 26, 2024

The bot is an org admin and I need it with such a role.
I've removed the bypass rule for org admins and enabled the bypass for repo maintainers.
Thereby, I've just made you a repo maintainer.

Not sure if this change will work. Need testing.

@pattacini pattacini merged commit 5081ba1 into robotology:devel Sep 26, 2024
1 check passed
@S-Dafarra
Copy link
Contributor

Awesome! I will test it immediately

@S-Dafarra
Copy link
Contributor

I rebased on top of devel and I force pushed. It wrote the following

Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
remote: Bypassed rule violations for refs/heads/devel-ergoCubSN001:
remote:
remote: - Cannot force-push to this branch
remote:
To https://github.com/icub-tech-iit/robots-configuration.git
 + a51affcbb...5081ba1fa devel-ergoCubSN001 -> devel-ergoCubSN001 (forced update)

In this case they should have been aligned, so it worked anyhow I suppose

@S-Dafarra
Copy link
Contributor

I tried to remove some commit and force pushed and it worked again, same message. Also in this case there were 0 deltas anyhow.

@pattacini
Copy link
Member Author

The message cannot force-push tells us that you cannot force-push when being identified as the bot doing that.
If you try out the same being yourself, we shouldn't get any failure.

@S-Dafarra
Copy link
Contributor

The message cannot force-push tells us that you cannot force-push when being identified as the bot doing that. If you try out the same being yourself, we shouldn't get any failure.

I did not enter my credentials, I suppose I pushed as bot

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.

5 participants