Skip to content

Reorder commands (toward Gitlab compatibility)#483

Merged
nuclearsandwich merged 2 commits intoros-infrastructure:masterfrom
locusrobotics:feature/gitlab_prepull_0
Sep 14, 2018
Merged

Reorder commands (toward Gitlab compatibility)#483
nuclearsandwich merged 2 commits intoros-infrastructure:masterfrom
locusrobotics:feature/gitlab_prepull_0

Conversation

@DLu
Copy link
Contributor

@DLu DLu commented Aug 28, 2018

In order to make #458 easier to merge, I've made this branch which just reorders some of the core commands such that

  • The content for the PR is created first, followed by the github specific logic.
  • The github is hidden behind a if True block, so that future whitespace changes are not needed.

Let me know if this helps @nuclearsandwich

@DLu DLu force-pushed the feature/gitlab_prepull_0 branch from 361a42b to 795cf21 Compare August 28, 2018 17:39
@DLu
Copy link
Contributor Author

DLu commented Aug 28, 2018

(the diff is much clearer with whitespace changes turned off)

@DLu
Copy link
Contributor Author

DLu commented Sep 3, 2018

Ping @nuclearsandwich

@nuclearsandwich
Copy link
Contributor

No whitespace link for other reviewers https://github.com/ros-infrastructure/bloom/pull/483/files?w=1

I am waiting on merging this to test the changes on an actual release. But I am generally positive about the change.

Copy link
Contributor

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

Tested locally and it works on python 2 and doesn't change the failure mode in python 3.

@nuclearsandwich nuclearsandwich merged commit ad3646b into ros-infrastructure:master Sep 14, 2018
@nuclearsandwich
Copy link
Contributor

Thanks for the continued work and for breaking things into review-able chunks.

DLu referenced this pull request in locusrobotics/bloom Sep 29, 2018
* Reorder commands

* Fix overly long line
@DLu DLu mentioned this pull request Dec 4, 2018
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