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

Add REP-144 check list to pr template, and split pull request template files #40518

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ijnek
Copy link
Contributor

@ijnek ijnek commented Apr 9, 2024

  • Adds REP-144 information and updates the DOC INDEX TEMPLATE
  • Since the PR template becomes rather large and readability is reduced, I have also split the pull request templates into mupltiple templates which can be selected from the root pr template.
  • Improvements to language and formatting to increase readability of templates.

Supercedes #39987.

@mjcarroll
Copy link
Member

Is the PULL_REQUEST_TEMPLATE feature new? I believe that we had tried this in the past without success, you had to put it all in a single markdown file

@ijnek
Copy link
Contributor Author

ijnek commented Apr 9, 2024

As far as I can tell, this feature has been available for a few years. One caveat is that GitHub lacks a user-friendly interface for selecting templates, unlike GitHub issue templates (eg. ros2-gbp's issue template selection).

Without a user-friendly interface, to access the other templates, one needs to utilize a query string in the URL (e.g., ?template=rosdep_rule_template.md). To circumvent this lack of interface, the root PR template is designed to include links to the other specialized PR templates.

Here's a screenshot of the root PR:

image

Copy link

This PR hasn't been activity in 14 days. If you are still are interested in getting it merged please provide an update. Otherwise it will likely be closed by a rosdistro maintainer following our contributing policy. It's been labeled "stale" for visibility to the maintainers. If this label isn't appropriate, you can ask a maintainer to remove the label and add the 'persistent' label.

@github-actions github-actions bot added the stale Issue/PR hasn't been active in a while and may be closed. label Apr 24, 2024
@ijnek
Copy link
Contributor Author

ijnek commented Apr 24, 2024

Please remove the 'stale' label, this is waiting for review

@github-actions github-actions bot removed the stale Issue/PR hasn't been active in a while and may be closed. label Apr 25, 2024
Copy link

github-actions bot commented May 9, 2024

This PR hasn't been activity in 14 days. If you are still are interested in getting it merged please provide an update. Otherwise it will likely be closed by a rosdistro maintainer following our contributing policy. It's been labeled "stale" for visibility to the maintainers. If this label isn't appropriate, you can ask a maintainer to remove the label and add the 'persistent' label.

@github-actions github-actions bot added the stale Issue/PR hasn't been active in a while and may be closed. label May 9, 2024
Copy link
Member

@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.

I'm in favor of this change.

I hatelove the "hack" of using the default pull request template to try and provide the user interface that GitHub won't.

This adds additional "steps" to the contribution process for ros/rosdistro but makes the pull request templates more legible. I am not a product manager so I don't know what conventional wisdom suggests this would do to our "funnel" for contributors. Before I approve and merge this, I want to bring it to a ROS 2 meeting.

@github-actions github-actions bot removed the stale Issue/PR hasn't been active in a while and may be closed. label Jun 5, 2024
Copy link

This PR hasn't been activity in 14 days. If you are still are interested in getting it merged please provide an update. Otherwise it will likely be closed by a rosdistro maintainer following our contributing policy. It's been labeled "stale" for visibility to the maintainers. If this label isn't appropriate, you can ask a maintainer to remove the label and add the 'persistent' label.

@github-actions github-actions bot added the stale Issue/PR hasn't been active in a while and may be closed. label Jun 19, 2024
@nuclearsandwich
Copy link
Member

Before I approve and merge this, I want to bring it to a ROS 2 meeting.

I haven't done this yet. It slipped my mind for the last ROS 2 meeting I attended. I've put it on the prospective agenda for the next ROS 2 meeting.


In the intervening weeks I realized that a more radical alternative would be to separate the official rosdep sources into their own repository, thus focusing the pull request guidelines and template on rosdistro operations, which may reduce the need to work around things in this manner. It seems like the gh CLI allows specifying a template but only from local files, not starting a browser tab with a selected PR template.

@quarkytale quarkytale added persistent Issue/PR won't be marked as stale if inactive for a while. and removed stale Issue/PR hasn't been active in a while and may be closed. labels Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
persistent Issue/PR won't be marked as stale if inactive for a while.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants