-
Notifications
You must be signed in to change notification settings - Fork 78
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
Update issue templates for new requirements #125
Changes from all commits
36decf9
ab48a68
26d74d8
5f311ef
3795705
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,8 +7,18 @@ title: 'Add new release repositories' | |
* Name of the release team: | ||
|
||
* Release repositories to add: | ||
<!-- | ||
Before release repositories are created, repositories and packages should be submitted to the official rosdistro repository https://github.com/ros/rosdistro for naming review. | ||
The ros/rosdistro source entry link must be included for each repository. | ||
Guidelines for package naming are describe in REP-144: https://www.ros.org/reps/rep-0144.html | ||
|
||
If there are any existing release repositories which should have their contents imported into the ros2-gbp organization, list and link to them here. | ||
Leave the checkbox _unchecked_. The ros2-gbp administrator will check the box when they have completed the repository import. | ||
--> | ||
* [FIRST_REPOSITORY](SOURCE URL) | ||
* [ros/rosdistro source entry](LINK TO SOURCE FIELD IN ROS/ROSDISTRO) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't understand what "LINK TO SOURCE FIELD" points to. Should a URL to a PR that is already opened in rosdistro be sufficient? With that, it'll be ros2-gbp maintainer's job to find the info that this line is currently asking for unfortunately, but as a total that might reduce confusion and make the process smoother. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree this step could step was confusing both from the instructions on docs.ros.org here:
I think it should suffice to have a link to an open PR otherwise the process could just take too long, waiting for maintainers of both ros/rosdistro and ros2-gbp/ros2-gbp-github-org There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback. Just opening the ros/rosdistro PR is not sufficient to indicate that a newly added repository has actually passed any kind of review, so this change to the process would not be sufficient and we would end up with the same cases which caused us to introduce the source repository requirement in the first place: packages being submitted which do not meet rosdistro naming requirements, needing a name change, and having to do that name change after having already created a ros2-gbp release repository. The current process is clunky and far from optimal. I have been working on a new bloom subcommand which automates submitted PRs for source repository indexing in the same fashion that it can currently automatically create release PRs and one potential feature of this project in the future is the automatic creation and deployment of ros2-gbp release repositories as soon as the source repository is added to ros/rosdistro. I appreciate the feedback immensely because it helps me better understand which areas of the process are the most in-need of optimizations. |
||
* [ ] There is an existing release repository which should be imported: <RELEASE REPOSITORY URL> | ||
* [SECOND REPOSITORY](SOURCE URL) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about making this 2nd repo entry option, or simply delete? This made me go through "why 2nd repo is asked here...Hm, maybe there are people who want to submit a single request for multiple repos, but I'm not sure how common that would be?", and finally I just made a single entry 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the feedback, I did assume that many "new" requests for ros2-gbp repositories would be in batches of two or more, but that probably only made sense when the ros2-gbp org project was new itself and adding repositories to an existing team probably is a one-at-a-time deal except in cases where a new suite of packages in separate repositories is being released for the first time. My general rule of thumb for templates is that you should show how to sequence things that can be duplicated and I would much rather one issue with two repositories than one issue per repository. |
||
* [ros/rosdistro source entry](LINK TO SOURCE FIELD IN ROS/ROSDISTRO) | ||
* [ ] There is an existing release repository which should be imported: <RELEASE REPOSITORY URL> | ||
* ... |
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.
Given a PR to
rosdistro
that lists up required info e.g. source repo URL is a must, shouldn't a link to such PR suffice? If so, we can get rid of[FIRST_REPOSITORY](SOURCE URL)
line, for simplification?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.
This is a great point! When the template was first written the rosdistro PR was not yet a requirement. It's slightly "more work" for the maintainer to have to follow the the source url through the rosdistro PR but I would much rather do that work in order to simplify the template! Thanks for the feedback!
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.
Thanks again for the feedback, I've incorporated it into a new draft PR here #296 but I'm also considering a larger restructuring of the documentation for release repository creation.