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

Cleanup specs and structure #329

Open
DanielOaks opened this issue Nov 9, 2017 · 11 comments
Open

Cleanup specs and structure #329

DanielOaks opened this issue Nov 9, 2017 · 11 comments
Labels
Milestone

Comments

@DanielOaks
Copy link
Member

DanielOaks commented Nov 9, 2017

This is something I've been meaning to do for a while, just making an issue so I remember to submit the changes. Once #327 is done and SASL is merged I'll do this.

  1. Merge CAP 3.1, CAP 3.2 and cap-notify (Merge CAP 3.1, CAP 3.2, and cap-notify specs #327). (done)
  2. Remove version numbers from spec filenames and YAML title names.
  3. Where they exist, remove markdown headers that duplicate the YAML title headers on specs (some specs for instance have an ## IRCv3 <blah> Extension up the top in addition to the YAML header – rip that out.
  4. Reorganise the specs under a few separate folders.
  5. Merge SASL 3.1 and SASL 3.2 (including @RyanSquared's PR extensions/sasl-3.1: Fix when to send 904 #408 ) and other points mentioned in the comments of this issue. (@grawity's changes here and comments here: grawity@317c2fa have been mostly-abandoned, prolly still worth looking through though)
  6. Make sure appropriate examples exist on all specs, so people implementing them have something to look at.
  7. We have a lot of encoding methods (cap values, tag values, isupport), and some of them are very similar (cap values and tag values). Split these out into a separate document if possible, so authors have to implement less code and can share them between uses.

This involves a lot of coordination between this repo and the website repo (renaming / combining specs data files, updating spec pages and etc), so need to be careful when merging PRs and subsequently pulling the updates into the website.

Since this repo's a submodule named /specs on the website, makes sense to have a few subfolders:

  • specs/client-tags: Client tag specifications.
  • specs/batches: Batch type specifications.
  • specs/sasl-mechs: SASL-mechanism-specific specs.
  • specs/metadata-keys: Metadata key specs.
  • specs/deprecated: Deprecated specs we want to keep around (such as the current SASL mech specs under specs/documentation).
  • specs/extensions: Everything not covered by the above.

Yep, under this arrangement everything currently in specs/core gets put into specs/extensions instead. It makes sure people know that things are optional (things like metadata aren't mandatory to implement to be IRCv3-compliant), and the main Specs page on the website is better positioned to make the necessary separation between, for example, cap, message-tags, and specs like extended-join anyway.

This replaces #265 and #299

@jwheare jwheare added the Meta label Nov 9, 2017
@jwheare jwheare mentioned this issue Nov 14, 2017
@DanielOaks
Copy link
Member Author

DanielOaks commented Nov 16, 2017

Note to self when SASL gets merged, fit this example into the merged spec:


Example showing new SASL mechanisms becoming available. This example requires the client to support CAP LS 302:

    Client: CAP LS 302
    Server: :irc.example.com CAP * LS :sasl=PLAIN
    ...
    Server: :irc.example.com CAP client DEL :sasl
    Server: :irc.example.com CAP client NEW :sasl=PLAIN,EXTERNAL

The given DEL/NEW messages may be displayed together for simplicity's sake.

@jwheare
Copy link
Member

jwheare commented Nov 16, 2017

We could also note that 908 can be used to advertise a change of mechlist

@DanielOaks
Copy link
Member Author

DanielOaks commented Nov 16, 2017

For changing mechanisms, should we push CAP DEL+NEW or 908? Maybe if they've already negotiated the sasl cap and the mechanism list changes, just send them 908 yeah? (imo it's prolly better to only push one rather than including both the CAP DEL+NEW and 908 methods and just saying "yeah choose whichever you want I guess?")

@jwheare
Copy link
Member

jwheare commented Nov 16, 2017

We could push 908 for changed mechlists but still have an example of sasl going away and then coming back on e.g. a services outage. Clients should handle both.

@DanielOaks
Copy link
Member Author

Yep, that sounds like a good way to split them up. I'll do it that way in the SASL merge then \o/

@jwheare
Copy link
Member

jwheare commented Apr 6, 2018

As mentioned on IRC. This should probably be a MUST:

If the client completes registration (with CAP END, NICK, USER and any other necessary messages) while the SASL authentication is still in progress, the server SHOULD abort it and send a 906 numeric, then register the client without authentication.

@grawity
Copy link
Contributor

grawity commented Apr 6, 2018

As long as the MUST phrasing doesn't accidentally forbid the server from rejecting unauthenticated clients

@DanielOaks
Copy link
Member Author

DanielOaks commented Apr 6, 2018

After the SASL specs get merged, we can throw another PR in that actually makes that change. Putting material changes in with the merge slows stuff down.

@jwheare
Copy link
Member

jwheare commented Apr 7, 2018

Fine by me!

@DanielOaks
Copy link
Member Author

Message-tags has also now been merged into a single spec. Whoo!

@DanielOaks
Copy link
Member Author

DanielOaks commented Feb 24, 2021

Reorganised the list of things-to-be-done. Much more action-oriented now.

  • 2/3/4) Pretty simple, just going through and changing and unifying file formats, so no biggie I'll get to it pretty soon.
  • 5) Merging SASL is a pretty big job, my time merging CAP taught me that there's a lot to keep in mind. It's a much shorter spec though so hopefully nowhere near as complicated. Don't touch SASL specs in 2+3+4 changes, since it's got an open nontrivial PR into it and all.
  • 6) Examples are hard. This'll be a spec-by-spec process. May involve some thoughts around how we display examples generally on the site / in our specs.
  • 7) Future me wants to do this. Current me does not and only sees pain down this path. Good luck to the future IRCv3 team of 2070 who manages to work on this.

Regarding sub-issues and etc, I don't think this needs to be split up. It was only blocked for so long because I didn't want to tangle with pending SASL changes that looked fairly widespread, but I've bumped that below general cleanups so that's not a blocker now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants