w3c / html-aria

ARIA in HTML
https://w3c.github.io/html-aria/
Other
181 stars 49 forks source link

Editorial: relocate change log #419

Closed marcoscaceres closed 2 years ago

marcoscaceres commented 2 years ago

Automatically generates the change log for a Proposed Edited Recommendation.


Preview | Diff

marcoscaceres commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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.

marcoscaceres commented 2 years ago

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 commented 2 years ago

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

marcoscaceres commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.