Add a section in the README for ADRs.#296
Add a section in the README for ADRs.#296bjohansebas wants to merge 5 commits intoexpressjs:masterfrom
Conversation
wesleytodd
left a comment
There was a problem hiding this comment.
I put request changes, but I think it is not truly something I consider blocking. I am just worried folks will be like me and need to go look up what "ADR" means to understand.
| @@ -0,0 +1,77 @@ | |||
| --- | |||
| name: ADR Template | |||
There was a problem hiding this comment.
Since I was not aware of the ADR format, I wonder if we could use more commonly used terminology here? RFC is the one I find most common and I know @sheplu even mentioned that on our call. That said, "proposal" would also make sense to me, and I think is more in the loose spirit of what these can be.
There was a problem hiding this comment.
an RFC is something different and I think we should prefer the semantically correct term for what this represents. RFC !== ADR
|
I was thinking about this PR, and I don't think it's best to include all the content of the ADR as the message of the PR, as it would just duplicate the content that the PR would carry. Perhaps we should provide a message stating 'use the template located in X place' and add a section in the README educating about the ADRs. What do you think? |
|
I think that is good idea @bjohansebas, so we only make changes in one place too. |
ab24d2a to
1a3d4f3
Compare
|
I don't see the need for a PR template for this. Functionally, unless it is the default PR template (named The value I do see here is adding an name: ADR Labeler
on:
pull_request:
types: [opened, synchronize]
paths:
- 'docs/adr/**'
jobs:
label-adr:
runs-on: ubuntu-latest
permissions:
pull-requests: write
steps:
- uses: actions/github-script@v7
with:
script: |
# Adding ADR label - if it already exists, this is a no-op
github.rest.issues.addLabels({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
labels: ['ADR']
}) |
|
I updated it to remove the template and fix the meetings section in the readme. |
| Current meeting schedule: | ||
|
|
||
| - TC Meeting: Monday 21:00 UTC (Every 2 weeks) | ||
| - Working Session: Wednesday 21:00 UTC (Every 2 weeks) |
There was a problem hiding this comment.
I wonder if we should just replace this with a link to the calendar or something? Either way, this seems fine to land as is now.
Related #291