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

Convert the specification into RFC format #466

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

jyasskin
Copy link

@jyasskin jyasskin commented Jan 6, 2025


on:
schedule:
- cron: '0 0 * * 0,2,4'
Copy link
Member

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?

Copy link
Author

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.

Copy link
Member

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.

Copy link
Author

Choose a reason for hiding this comment

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

Done: #467

CONTRIBUTING.md Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
draft-marchan-kdl2.md Show resolved Hide resolved
Copy link
Member

@zkat zkat left a 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


on:
schedule:
- cron: '0 0 * * 0,2,4'
Copy link
Author

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.

@@ -0,0 +1,36 @@
name: "Update Generated Files"
Copy link
Author

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.

CONTRIBUTING.md Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
draft-marchan-kdl2.md Show resolved Hide resolved
Copy link
Member

@zkat zkat left a 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?


### 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
Copy link
Member

Choose a reason for hiding this comment

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

Two things:

  1. Looks like this specific instance is missing the section link. Perhaps there are other cases?

  2. On a more general note, I notice that there's some inconsistency as to events we do [Foo](#foo) ([Section 1](#section1)) or just Foo ([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.

Copy link
Author

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.

draft-marchan-kdl2.md Outdated Show resolved Hide resolved
draft-marchan-kdl2.md Outdated Show resolved Hide resolved
draft-marchan-kdl2.md Outdated Show resolved Hide resolved
draft-marchan-kdl2.md Outdated Show resolved Hide resolved
draft-marchan-kdl2.md Outdated Show resolved Hide resolved
draft-marchan-kdl2.md Outdated Show resolved Hide resolved
draft-marchan-kdl2.md Outdated Show resolved Hide resolved
draft-marchan-kdl2.md Outdated Show resolved Hide resolved
draft-marchan-kdl2.md Outdated Show resolved Hide resolved
Copy link
Member

@zkat zkat left a 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 :)

@zkat
Copy link
Member

zkat commented Jan 8, 2025

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

Copy link
Contributor

@danini-the-panini danini-the-panini left a 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?

Comment on lines +34 to +36
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`.
Copy link
Contributor

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

@zkat
Copy link
Member

zkat commented Jan 9, 2025

Also, do we need to rewrite the full grammar to use ABNF as described in RFC5234?

@danini-the-panini #461 (comment)

tl;dr no, but they might ask us to formalize our DSL later, maybe.

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.

Write spec in IETF RFC style
3 participants