-
Notifications
You must be signed in to change notification settings - Fork 74
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
Conversation
Lots of folks have been checking the boxes here when they submit the template, however my intention was that they would populate the incomplete checklist and the ros2-gbp admins would check those off when they import the repo. We'll see if this comment improves the situation or not. If it doesn't I may remove the checklist from the template entirely.
As discusesed, we want repositories to undergo naming and license review as source packages before we create release repositories for them. Right now this process is quite manual but building tools which streamline the workflow is planned.
190c9b2
to
5f311ef
Compare
@ijnek what do you think of these changes? I think we need to enhance the contributing docs in ros/rosdistro with some explicit cases for this as well but I started these updates first. |
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 for adding this, these changes look good!
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.
Left some feedback for the template based on the conversation in #275
* [FIRST_REPOSITORY](SOURCE URL) | ||
* [ros/rosdistro source entry](LINK TO SOURCE FIELD IN ROS/ROSDISTRO) | ||
* [ ] 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 comment
The 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 comment
The 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.
|
||
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) |
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.
* [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 comment
The 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 comment
The 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:
- https://docs.ros.org/en/rolling/How-To-Guides/Releasing/Release-Team-Repository.html#create-a-new-release-repository
and from this template.
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 comment
The reason will be displayed to describe this comment to others. Learn more.
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
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.
This is a start at addressing some of the feedback provided in #125 My objective in using a big issue template with lots of explanatory comments was to make the documentation as local to its use as possible without cluttering the resulting issue (after all, the submitter can't both read the formatted text and fill out the template at the same time). But as I iterate, I'm starting to think that [Issue Forms](https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/syntax-for-issue-forms) and a link to guiding documentation would be easier to follow and result in stil cleaner issues than this format. So I may abandon this effort in pursuit of a wider refactor.
Related to #106.