Skip to content

Use index for querying distro information#171

Merged
tfoote merged 1 commit intoros-infrastructure:masterfrom
lgsvl:dashing
Apr 11, 2019
Merged

Use index for querying distro information#171
tfoote merged 1 commit intoros-infrastructure:masterfrom
lgsvl:dashing

Conversation

@andre-rosa
Copy link
Contributor

@allenh1
Copy link
Contributor

allenh1 commented Apr 9, 2019

Lgtm. Leaving for @tfoote to merge

@allenh1 allenh1 self-requested a review April 9, 2019 12:35
allenh1
allenh1 previously approved these changes Apr 9, 2019
@allenh1 allenh1 requested a review from tfoote April 9, 2019 12:36
@nuclearsandwich
Copy link
Contributor

Now that the index-v4.yaml file includes information about ROS 1 vs ROS 2 distributions. Is it possible to use the index rather than hard-coding distribution lists in the tool?

@nuclearsandwich
Copy link
Contributor

In fact, looking more closely at the source I'm not sure that the commit message formatting even merits working that hard. And it makes it harder for orgs with forked rosdistros to use superflore.

@nuclearsandwich
Copy link
Contributor

I've opened #172 which removes the hard coded distro lists from commit messages but couldn't quickly replace the last hard coded list. I still believe it's possible to do so and have left details on the other PR.

@andre-rosa
Copy link
Contributor Author

andre-rosa commented Apr 9, 2019

I've opened #172 which removes the hard coded distro lists from commit messages but couldn't quickly replace the last hard coded list. I still believe it's possible to do so and have left details on the other PR.

Great @nuclearsandwich @allenh1 , thanks! I pushed a change on top of yours (depends on #172) adding the support to get the hardcoded information from the index instead. Could you please take a look?

@andre-rosa andre-rosa changed the title Add Dashing Diademata ROS 2 release Use index for querying distro information Apr 9, 2019
tfoote
tfoote previously approved these changes Apr 10, 2019
Copy link
Member

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

@andre-rosa Thanks for filling in the rosdistro index logic! It looks good. Though I merged #172 before realizing that this had integrated it too but apparently this needs a rebase to merge. Otherwise it lgtm.

@andre-rosa
Copy link
Contributor Author

@tfoote << Thank you for reviewing! I rebased it on top of the dependent changes. Could you please take another look?

@tfoote tfoote merged commit a637cf1 into ros-infrastructure:master Apr 11, 2019
@andre-rosa
Copy link
Contributor Author

Thanks, I'll rebase now!

@andre-rosa andre-rosa deleted the dashing branch April 11, 2019 01:38
@allenh1 allenh1 mentioned this pull request Apr 11, 2019
zffgithub pushed a commit to zffgithub/superflore that referenced this pull request Apr 11, 2023
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.

4 participants