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.

Sign up for GitHub

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

Contributing update README.md #108

Closed
wants to merge 5 commits into from
Closed

Contributing update README.md #108

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ef7bdf4
Update README.md
ColmBhandal Aug 30, 2023
2258dba
Update README.md
ColmBhandal Aug 30, 2023
232d6d0
Update actions/contributing-update/README.md
ColmBhandal Sep 1, 2023
8d437a1
Update actions/contributing-update/README.md
ColmBhandal Sep 1, 2023
e9ee044
Merge branch 'main' into contributing-update-readme
ColmBhandal Sep 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions actions/contributing-update/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,24 @@
### Contributing Workflow
# Contributing Workflow

This action checks the contributing file for a charm and updates it if necessary. The action runs a script that generates a fresh contributing file from a template and placeholder values defined at the charm level. This generated file is compared against the contributing file already in the charm's directory. If they differ, another step is run to create a PR overwriting the existing file with the freshly generated one.

### contributing_inputs.yaml
## contributing_inputs.yaml

This file, existing in charm's directory, defines all charm-specific inputs that will be injected into the `contributing.md.template` file to form the new `contributing.md` file. If this file doesn't exist, or doesn't contain definitions for all template values, the contributing update will fail, as there won't be enough information to generate the new `contributing.md` file from the template.
This file, existing in charm's directory, defines all charm-specific inputs that will be injected into the `contributing.md.template` file to form the new `contributing.md` file. If this file doesn't exist, or doesn't contain definitions for all template values, the contributing update will fail, as there won't be enough information to generate the new `contributing.md` file from the template.

## Updates

### Charm Specific

Whenever we want to make changes to a charm `contributing.md`, we:

1. Update `contributing_inputs.yaml`
1. We can also update the `contributing.md` file directly if we like, if we want to avoid another PR
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it'd make more sense to discourage people from editing it directly? Sort of defeats the purpose of this automation IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I do. I think this is a hacky use case as you mention in your other comment. I think the follow-on issue will address that, and the README will need to be updated when it's done.

For now though, I think it's best to document the action as it is today, seeing as it's already merged and this is how it actually works.

1. Push our changes
1. If the freshly generated `contributing.md`, based on the updated `contributing_inputs.yaml` plus the `contributing.md.template`, differs from `contributing.md`, the automation will catch this, and a PR will be opened to overwrite the existing `contributing.md` with the new one.

Note: the generate PR will target the `main` branch, BUT the target branch could always be changed to the same branch as the one updating `contributing_inputs.yaml`, and then merged into that without review, to avoid having two PRs to review.
Copy link
Contributor

Choose a reason for hiding this comment

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

Come on, even describing this feels super hacky 😂 I'd be all for setting this behaviour as the default though. I believe it's much better than either encouraging people to edit the contributing.md directly or manually directing the rendered file PR towards the one updating the inputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm. You're right. I think it would be as simple as this: instead of targeting main, just target "Whatever branch you are currently on".

However, targeting main is how it works today :(

I really don't want to allow the scope of this PR to extend beyond simply updating the README to reflect the state of the already-merged action.

So, I've created an issue to change the action as you suggested: #109.

And then for this PR, I suggest we focus just on documenting the action as it is today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I may be wrong. I'm testing this out. I see from the PR action we're using that the default branch is the one from which the action is called:

base Sets the pull request base branch. Defaults to the branch checked out in the workflow.

So I'm testing this out in this workflow run.


### Template Updates

Even if the charm-specific `contributing_inputs.yaml` has not changed, it is possible that `contributing.md.template` will change over time. In this case, all repositories using this workflow should re-run it. It is advisable, therefore, for such repositories to ensure they are running this workflow on a regular schedule, e.g. weekly, to detect such updates.