-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
Convert the specification into RFC format #466
base: main
Are you sure you want to change the base?
Conversation
This works around cabo/kramdown-rfc#249.
RFCXML v3 doesn't include horizontal rules.
That compiles to <sourcecode> which preserves sequences of spaces.
.github/workflows/archive.yml
Outdated
|
||
on: | ||
schedule: | ||
- cron: '0 0 * * 0,2,4' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is because IETF considers all discussions and repo history as normative or such?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to keep all of the actions if you don't want one of them. Discussion wouldn't be normative, but it's useful to have them around later. But for an archive of GitHub content to be useful, it would have to be stored away from GitHub, which this isn't, so I don't really see the point. It just comes with the default setup. Let me know if you'd rather I delete it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can skip doing this for now, then, until we actually figure out an external storage system. If you decide to do that, though, please file an issue about it so we don't completely forget to do it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: #467
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops I didn’t mean to approve
For if installing the dependencies is inconvenient.
.github/workflows/archive.yml
Outdated
|
||
on: | ||
schedule: | ||
- cron: '0 0 * * 0,2,4' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to keep all of the actions if you don't want one of them. Discussion wouldn't be normative, but it's useful to have them around later. But for an archive of GitHub content to be useful, it would have to be stored away from GitHub, which this isn't, so I don't really see the point. It just comes with the default setup. Let me know if you'd rather I delete it.
.github/workflows/update.yml
Outdated
@@ -0,0 +1,36 @@ | |||
name: "Update Generated Files" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading this one, it's actually dangerous enough that I've deleted it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized halfway through reviewing this that the places where I said "missing section link) ARE actually getting sections, but that those are the places where both the name and the section are links.
So maybe we can just turn those into Foo ([Section1](...))
for consistency?
draft-marchan-kdl2.md
Outdated
|
||
### Document | ||
## Document | ||
|
||
The toplevel concept of KDL is a Document. A Document is composed of zero or | ||
more [Nodes](#node), separated by newlines and whitespace, and eventually |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things:
-
Looks like this specific instance is missing the section link. Perhaps there are other cases?
-
On a more general note, I notice that there's some inconsistency as to events we do
[Foo](#foo) ([Section 1](#section1))
or justFoo ([Section 1](...))
. I would prefer we normalize around one. The second (just one link) probably makes the most sense, since linking the name itself seems redundant if you're going to link the section anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, this is cabo/kramdown-rfc#249. Works for me to use just the Foo ({{foo}})
link structure. I'm a little nervous that'll be so many changes that it breaks git's move-detection, but if it does, we can always split it into a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, but I'd like to let others get a chance to weigh in on the idea of doing this, before we go through with it.
Additionally, I note that there's a particular copyright notice, and I'm confused as to whether it needs modification or it just applies to the "boilerplate": https://trustee.ietf.org/about/faq/#reproducing-rfcs
We would, of course, like the copyrights to belong to KDL Contributors. One small snag might be that the contents of this repository are licensed under CC-BY-SA-4.0, which is definitely a more restrictive license than the IETF seems to expect (with good reason!!). If we are to change the license situation, we'll definitely want to make sure we know what we need to relicense to and then start the relicensing process, which might take a while, since it requires consent from every single person who has ever contributed to this repo. I believe we can cross that bridge when we get to it, though. I personally like the RFC format regardless of whether the IETF takes us on :)
Cc @tabatkins @borland @bgotink @larsgw @danini-the-panini and of course whoever else wants to chime in. We should probably set up an org team so we can ping one specific group for anyone interested in spec discussions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also seeing a lot of "line too long warnings". I'm not sure if that's a deal-breaker, but I'll make a PR to try address them.
Also, do we need to rewrite the full grammar to use ABNF as described in RFC5234?
https://author-tools.ietf.org/ or locally by running `make`. To preserve the | ||
intermediate RFCXML form in a local build, `touch draft-marchan-kdl2.xml` before | ||
running `make`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried this and I just got an error
❯ touch draft-marchan-kdl2.xml
❯ make
Error: Unable to parse the XML document: draft-marchan-kdl2.xml
draft-marchan-kdl2: xml2rfc-txt ... FAIL
make: *** [draft-marchan-kdl2.txt] Error 1
@danini-the-panini #461 (comment) tl;dr no, but they might ask us to formalize our DSL later, maybe. |
Fixes #461.
You can see a preview at https://jyasskin.github.io/kdl/draft-marchan-kdl2.html.