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

Breakage #1

Closed
8 tasks done
annevk opened this issue Mar 31, 2020 · 4 comments
Closed
8 tasks done

Breakage #1

annevk opened this issue Mar 31, 2020 · 4 comments

Comments

@annevk
Copy link
Member

annevk commented Mar 31, 2020

Currently there are the following issues. Since we don't run this often it's unclear whether they need to be resolved.

  • Compatibility uses compatibility.bs while its shortname is compat.
  • Makefile.template does not support (do we really only need these for deploy, not review?)
    • EXTRA_FILES
    • POST_BUILD_STEP
  • Console
    • Different .editorconfig (indent_size = 2 rather than 1)
    • Different .gitignore
    • Has no .pr-preview.json
    • Custom Makefile (does not seem to be needed other than EXTRA_FILES though)
  • Encoding needs /UnicodeData.txt in .gitignore.
  • HTML is mostly incompatible as expected
    • Weird that it has indent_size = 2 and no max_line_length = 100.
    • No .gitignore. Probably because building is done elsewhere.
  • Streams
    • Minor .editorconfig mismatches again.
    • Needs `.gitignore additions
    • Has "post_processing" member in .pr-preview.json, but also seems to have some errors there (e.g., does not overwrite h1).
    • Makefile is custom as there's a node dependency.

Overall it works pretty well though (for 10 out of 15, and for the remaining 5 it still helps identifying issues) and ends up adding PULL_REQUEST_TEMPLATE.md to a bunch of places as well as making minor ordering changes to .gitignore.

@domenic
Copy link
Member

domenic commented Mar 31, 2020

Compatibility uses compatibility.bs while its shortname is compat.

We could rename it. If we do it in a single commit history will still be there in Git, although it looks like the GitHub UI does not follow renames :(. So, maybe not worth it... I guess we could hard-code an exception into the tool, if we anticipate this being a long-term issue.

Console: Different .editorconfig (indent_size = 2 rather than 1)

We should fix the editorconfig and the spec to align.

Console: Different .gitignore

This seems to be due to the test/ and reference-implementation/ subdirectories. However those have not been maintained, so we should just remove it. See also whatwg/console#152.

The other divergences can be removed.

Console: Has no .pr-preview.json

We should fix

Console: Custom Makefile (does not seem to be needed other than EXTRA_FILES though)

We should align it. Although, we could copy some of its additions, which are reasonable practice:

SHELL=/bin/bash -o pipefail
.PHONY: local remote deploy review

(See here for the .PHONY business.)

HTML: Weird that it has indent_size = 2 and no max_line_length = 100.

Oh wow, that's why my editor keeps resetting to indent_size 2!

Streams

I would like to remove Node from the build process at some point, and align on indent/line length.

@annevk
Copy link
Member Author

annevk commented Apr 1, 2020

It now works for everything except HTML and Streams. I'll still PR Streams with some local tweaks as overall it's an improvement there too.

@annevk
Copy link
Member Author

annevk commented Apr 22, 2020

a10cf79 excludes HTML/Streams. I suspect Streams can soon be enabled again and for HTML we probably want to to update a subset of files only.

@annevk
Copy link
Member Author

annevk commented Oct 12, 2022

Streams was fixed in #15.

@annevk annevk closed this as completed Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants