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

Update pr instructions GHA with URL for reviewing changes to CONTRIBUTING.md #5165

Closed
5 tasks
Tracked by #5182
roslynwythe opened this issue Aug 8, 2023 · 4 comments · Fixed by #5438
Closed
5 tasks
Tracked by #5182

Update pr instructions GHA with URL for reviewing changes to CONTRIBUTING.md #5165

roslynwythe opened this issue Aug 8, 2023 · 4 comments · Fixed by #5438
Assignees
Labels
Complexity: Medium Feature: Wiki role: back end/devOps Tasks for back-end developers size: 2pt Can be done in 7-12 hours time sensitive Needs to be worked on by a particular timeframe

Comments

@roslynwythe
Copy link
Member

roslynwythe commented Aug 8, 2023

Overview

When a Pull Request includes changes to CONTRIBUTING.md, the current PR instructions are not accurate, because CONTRIBUTING.md cannot be previewed locally, rather it must be previewed on a GitHub repository. We need to instruct the PR author and reviewers to preview changes using the URL of CONTRIBUTING.md on the issue branch in the contributor's fork of the website repository.

Details

Here is the new instruction which should be appended to the existing instruction:

Note that CONTRIBUTING.md cannot previewed locally; rather it should be previewed at this URL:

https://github.com/[REPLACE WITH nameOfCollaborator]/website/blob/[NAME OF FROM BRANCH]/CONTRIBUTING.md

where nameOfCollaborator and nameOfFromBranch refer to the variables with the same names in create-instruction.js1.
The text above should be stored in a new template github-actions/pr-instructions/pr-contrib-instructions-template.md and the variable replacement should be done in create-instruction.js2 as detailed below.

The suggested strategy is to reformat the JS by moving the formatComment function into create-instruction.js so that regardless of how many instructions we provide in the PR (each with its own template), only a single composite instruction will be stored in the artifact, and in create-instruction.js, we have access to the PR context so that in the future, we could include or exclude instructions based on the files that were changed.

Action Items

  • Become familiar with the resources describing the HfLA GHA archtecture2 and testing procedures3 as well as the resource describing the PR Instructions GHA4 and understand the scripts create-instruction.js1 and post-comment.js5
  • The recommended approach is to update create-instruction.js1 so that it contains a larger instruction composed of two parts:
    • part 1 is the current pr instruction, but now it will be created in create-instruction.js1 by merging the existing instruction string into the existing pr-instructions-template.md6. Previously this step was performed in formatComment() so that function will have to be moved from post-comment.js5 into create-instruction.js7
    • part 2 is the new portion described in the Details section above. It will be created by merging a new string previewContributingURL into the new template.
  • To propose an alternative approach, discuss with merge team/ dev lead

Resources/Instructions

Footnotes

  1. https://github.com/hackforla/website/blob/gh-pages/github-actions/pr-instructions/create-instruction.js 2 3 4

  2. Hack for LA's GitHub Actions 2

  3. HfLA GitHub Actions

  4. GHA: pr instructions

  5. https://github.com/hackforla/website/blob/gh-pages/github-actions/pr-instructions/post-comment.js 2

  6. pr-instructions-template.md

  7. PR Instructions Template

@roslynwythe roslynwythe added role: back end/devOps Tasks for back-end developers Complexity: Large Feature: Wiki size: 2pt Can be done in 7-12 hours Draft Issue is still in the process of being created labels Aug 8, 2023
@roslynwythe roslynwythe changed the title Update pr instructions GHA with special guidance for reviewing changes to CONTRIBUTING.md Update pr instructions GHA with guidance for reviewing changes to CONTRIBUTING.md Aug 8, 2023
@roslynwythe roslynwythe changed the title Update pr instructions GHA with guidance for reviewing changes to CONTRIBUTING.md Update pr instructions GHA with URL for reviewing changes to CONTRIBUTING.md Aug 8, 2023
@roslynwythe roslynwythe added ready for product and removed Draft Issue is still in the process of being created labels Aug 8, 2023
@roslynwythe roslynwythe added Draft Issue is still in the process of being created Ready for Prioritization and removed Ready for Prioritization Draft Issue is still in the process of being created labels Aug 10, 2023
@wanyuguan wanyuguan added this to the 08. Team workflow milestone Aug 12, 2023
@roslynwythe
Copy link
Member Author

roslynwythe commented Aug 29, 2023

I suggest adding time-sensitive to this issue, because we have quite a few changes to CONTRIBUTING.md coming up, and it would be nice to have these enhanced instructions for devs to test those changes.

I self-assigned it and went ahead with the changes. It can be previewed at roslynwythe#40

@roslynwythe roslynwythe added time sensitive Needs to be worked on by a particular timeframe Complexity: Medium labels Aug 29, 2023
@roslynwythe roslynwythe self-assigned this Aug 31, 2023
@github-actions
Copy link

Hi @roslynwythe, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:-
i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?)
ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)

@roslynwythe
Copy link
Member Author

The problem is that the pr instruction contains backticks and those cause a problem in github artifacts

@ExperimentsInHonesty
Copy link
Member

@roslynwythe If you are working on this issue, go ahead and remove the ready for dev lead label and put it in the In progress column.

@ExperimentsInHonesty ExperimentsInHonesty added ready for dev lead Issues that tech leads or merge team members need to follow up on and removed Ready for Prioritization labels Oct 2, 2023
@roslynwythe roslynwythe removed the ready for dev lead Issues that tech leads or merge team members need to follow up on label Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment