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

Editorial: relocate change log #419

Merged
merged 3 commits into from
May 31, 2022
Merged

Editorial: relocate change log #419

merged 3 commits into from
May 31, 2022

Conversation

marcoscaceres
Copy link
Member

@marcoscaceres marcoscaceres commented May 19, 2022

Automatically generates the change log for a Proposed Edited Recommendation.


Preview | Diff

@marcoscaceres
Copy link
Member Author

marcoscaceres commented May 19, 2022

@scottaohara, @stevefaulkner, this automatically filters out any commit message that, as we discussed earlier on, is not prefixed with "addition:" and "correction:".

However, it looks like for some, you have forgotten to label some (ok.. most 😔) commits as either a correction: or an addition:. It's critical you do that.

You have some options:

  1. Rewrite the javascript filter to append "correction:" or "addition:" to the commit message based on the commit hash.
  2. Do a git rebase -i and append each commit entry with "correction:" or "addition:" (and push all the rewritten history back up to the server).
  3. Go back to the manual change log, and add "correction:" / "addition:", but you MUST link to the pull request.
  4. ... or, throw our hands in the air, and we go back to Candidate Recommendation and go through the whole Recommendation dance again.

@marcoscaceres
Copy link
Member Author

FWIW, I think 2 is the most viable.

It will, however, screw up anyone who has forked or cloned this repo, as it will break their history (any pull request will no longer apply). They will need to do a clean clone of the repo.

@scottaohara
Copy link
Member

scottaohara commented May 19, 2022

not so much "forgotten" as didn't realize this was necessary, and have already made a manual commit log that references and links to every normative change in the spec. The preview/diff builds don't show the new change log, even though i do have some commits that were made that are prefaced with "correction:" and "addition:".

is this necessary to implement for this version, @marcoscaceres if everything is already called out in the current changelog? It'd be far easier to just do option 3 at this point (and have just pushed a change to do so). I recall you mentioning that manifest uses the automatic changelog, but it doesn't reference the date of the change log, and since it just points to the PR/commits, it doesn't help easily find (link to) the changed content in the actual spec. That seems unfortunate to lose compared to what I've done here.

scottaohara added a commit that referenced this pull request May 19, 2022
directly reference the following PRs in the changelog, and prepend each entry with the appropriate 'label'

related to #419

- #352
- #367 (and follow on [commit](887c772))
- #360
- #353
- #372
- #402
- #404
- #403
- #396
- #391
- #369
- #381
@marcoscaceres
Copy link
Member Author

marcoscaceres commented May 20, 2022

if everything is already called out in the current changelog?

Ok, so what would need to change in the changelog, you need to put if it's a "correction:" or an "addition", and link to the relevant pull request for each. Linking to were the change is made is also a helpful touch.

So, for example:

  <li>
     Correction:
     <a href="#att-checked">`aria-checked`</a> is not to be used on elements that support the `checked` attribute.
   </li> (<a href="https://github.com/link-to-pull-request">pull request #123</a>)

And move the changelog to the Status of This Document, as I've done in this pull request.

I recall you mentioning that manifest uses the automatic changelog, but it doesn't reference the date of the change log, and since it just points to the PR/commits, it doesn't help easily find (link to) the changed content in the actual spec. That seems unfortunate to lose compared to what I've done here.

That's correct. I figured linking to changes is overkill, because once the document gets approved, all the boxes go away. The change log itself should be coming from the commit messages viewed on Github (or should be getting automatically included via <rs-changelog>).

For example, this is what it looks like when the git commit history is clean:
https://www.w3.org/TR/payment-request/#changelog

It's nice and flat and each commit clearly describes what's changed. Then the details and and diff (including HTML diff) are provided via Github.

@marcoscaceres
Copy link
Member Author

@scottaohara, I moved the updated change log into the Status section. Let's try going with this.

@marcoscaceres marcoscaceres requested a review from scottaohara May 26, 2022 07:45
@marcoscaceres
Copy link
Member Author

@scottaohara, this should be good to go, yes? This, together with #418 gets us back on /TR/ and if you and @stevefaulkner are ready, we can request the AC to review (so we can publish as an updated REC).

@scottaohara
Copy link
Member

@marcoscaceres have been incredibly busy the last week so have not had a chance to pull this down to do a proper review yet (the preview/diff pages don't show the new content, so doing a check there is unfortunately useless).

as an fyi, there is still a bit more work that needs to be done before we're ready for this to go up for review again. but i will try to get that sorted in the next week or so and then let you/leonie know when we're good to go.

@marcoscaceres
Copy link
Member Author

as an fyi, there is still a bit more work that needs to be done before we're ready for this to go up for review again.

That's totally fine (take as long as you need). However, the point of this PR is to get us auto-publishing to /TR/ again. This doesn't trigger any review by the AC or anything like that.

@marcoscaceres
Copy link
Member Author

marcoscaceres commented May 27, 2022

Just to be clear, we should be able to merge this as is. This is just to restart the automatic publishing system. That's all.

All I did was relocate your Change Log to the Status of This Document.

Then, a few months down the line, when you are ready, we can do another proper REC transition.

@scottaohara
Copy link
Member

scottaohara commented May 27, 2022

oh, i see... the changelog is at the top of the document now? hmm... i'm not seeing any other specs do this.

what if this was put within a <details><summary>? lines 63 to 123?

@marcoscaceres
Copy link
Member Author

marcoscaceres commented May 31, 2022

oh, i see... the changelog is at the top of the document now? hmm... i'm not seeing any other specs do this.

That's because we would be the first ever PER (Proposed Edited Recommendation).
https://www.w3.org/TR/?status=PER

what if this was put within a <details><summary>? lines 63 to 123?

That kinda defeats the purpose. The point is to show clearly to the community what the additions and corrections are before they are incorporated.

@marcoscaceres marcoscaceres changed the title Editorial: auto-generate changelog Editorial: relocate change log May 31, 2022
@marcoscaceres
Copy link
Member Author

I'm going ahead and merging this, as this is just W3C Process related and I'd like to get this back on the TR. This doesn't affect editorial work.

@marcoscaceres marcoscaceres merged commit 0a833d0 into gh-pages May 31, 2022
@marcoscaceres marcoscaceres deleted the changelog branch May 31, 2022 03:03
pkra pushed a commit to w3c/aria that referenced this pull request May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants