[go: nahoru, domu]
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

Jump to bottom

Assert ids for version 1.2.0 and draft 1.3.0 #171

Merged
merged 13 commits into from
Dec 11, 2023
Merged

Assert ids for version 1.2.0 and draft 1.3.0 #171

merged 13 commits into from
Dec 11, 2023

Conversation

podborski
Copy link
Collaborator
@podborski podborski commented May 15, 2023 •
edited by pr-preview bot
Loading
  • finish editing index_120.bs
  • generate new v1.2.0.html
  • delete index_120.bs and other intermediate files

Preview | Diff

@podborski podborski linked an issue May 15, 2023 that may be closed by this pull request
Add assert-ids to version 1.2.0 of the spec #169
Closed
@cconcolato
Copy link
Collaborator

We need to backport this change in the Bikeshed file 7f97ee2

@podborski
Copy link
Collaborator Author

Hi @cconcolato can you take a look at the index_120.bs? We have some troubles with bikeshed as it generates non valid html. (I see missing closing tags etc.) We also tried to run on the old version of the bikeshed and it still generates more changes than expected to the output html.

@cconcolato
Copy link
Collaborator

Have a look at http://services.w3.org/htmldiff

@cconcolato
Copy link
Collaborator
cconcolato commented Jun 12, 2023 •
edited
Loading
  1. Generate the HTML of 1.2 with unmodified Bikeshed file but with the latest Bikeshed software. Confirm that there is no meaningful difference
  2. Add asserts to the 1.2 Bikeshed file (including unicity hacks), generate HTML
  3. Update 1.3 Bikeshed to fix the unclosed assert and to guarantee unicity of assert ids
  4. Make sure we use only one branch for published and draft specs (no gh-pages anymore), that would solve Improve publishing approach #164

podborski and others added 2 commits June 26, 2023 09:29
@podborski
run the latest bikeshed version on the updated .bs file
61b1746
@podborski
run bikeshed 3.11.21 on v120
3cf8fa8 
index120_init.bs: initial BS file for v120
v120_init.html: compiled index120_init.bs
v120_main.html: currently published v120
v1.2.0.html: compiled modified index120_init.bs (with assert ids)
@podborski
Copy link
Collaborator Author
  1. Generate the HTML of 1.2 with unmodified Bikeshed file but with the latest Bikeshed software. Confirm that there is no meaningful difference

v120_main.html vs v120_init.html diff

Looks like we only have to slightly edit the header.

  1. Add asserts to the 1.2 Bikeshed file (including unicity hacks), generate HTML

v120_init.html vs v1.2.0.html diff

the diff tool shows 130 changes but all of them are due to inserted assert id's. (content remains the same).

@cconcolato
Copy link
Collaborator

@podborski fyi speced/bikeshed#2686

@podborski podborski mentioned this pull request Oct 19, 2023
Merged
@podborski podborski marked this pull request as ready for review November 27, 2023 17:18
@podborski
Copy link
Collaborator Author

I believe the diff tool has an issue with the above hidden span hack we use:
image
It shows that the fields are removed (left side of the figure) while the generated html (right side of the figure) still shows that the fields are there.

@cconcolato
Copy link
Collaborator

We noticed that spans are closed with a p. This should be fixed.

@cconcolato cconcolato changed the title Assert ids for version 1.2.0 Assert ids for version 1.2.0 and draft 1.3.0 Nov 27, 2023
@podborski
Copy link
Collaborator Author

@cconcolato I believe its now good to go for your review. fixed the closing span's and merged the latest main branch (and added a changelog entry).

cconcolato
cconcolato approved these changes Nov 29, 2023
View reviewed changes
Copy link
Collaborator
@cconcolato cconcolato left a comment

Choose a reason for hiding this comment

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

I verified the v1.2.0.html in this PR against the one currently published. No meaningful differences.
I verified the HTML generated online from the index.bs in this PR against the latest draft. No meaningful differences.
@podborski you can merge after removing the extra files (index120_init.bs, index_120.bs and v120_*.html).

@cconcolato
Copy link
Collaborator

@podborski can you clean the extra HTML AND Bikeshed files?

@cconcolato cconcolato merged commit df5721b into main Dec 11, 2023
@cconcolato cconcolato deleted the assert_ids_120 branch December 11, 2023 16:54
github-actions bot added a commit that referenced this pull request Dec 11, 2023
@cconcolato @github-actions
Merge pull request #171 from AOMediaCodec/assert_ids_120
16dfb75 
SHA: df5721b
Reason: push, by cconcolato

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cconcolato cconcolato cconcolato approved these changes

@ruegfo ruegfo ruegfo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add assert-ids to version 1.2.0 of the spec
3 participants
@podborski @cconcolato @ruegfo