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

feat: Add idnits3 for post submission nits check button #8169

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

Conversation

kesara
Copy link
Member

@kesara kesara commented Nov 6, 2024

This change adds a new Nits 3 button in addition to the existing Nits button.

Screenshot 2024-11-19 at 13 56 27

Fixes #8107

@@ -2996,6 +2996,24 @@ def test_raw_id(self):
self.should_succeed(dict(name=draft.name, rev='00',ext='txt'))
self.should_404(dict(name=draft.name, rev='00',ext='html'))

@override_settings(IDNITS3_BASE_URL="https://example.org/idnits3")
def test_nits_url(self):
draft = WgDraftFactory(name="draft-ietf-moon-test",rev="01", create_revisions=range(0,2))
Copy link
Member

Choose a reason for hiding this comment

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

A comment as to why you're setting up this particular arrangement of revisions would be helpful

Copy link
Member

@jennifer-richards jennifer-richards left a comment

Choose a reason for hiding this comment

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

Oops, meant to request changes for the comment (code itself looks fine)

@rjsparks
Copy link
Member

rjsparks commented Nov 7, 2024

If someone really wanted the v2 nits, can they get back to them quickly?

@rjsparks
Copy link
Member

rjsparks commented Nov 14, 2024

Nudge:

If someone really wanted the v2 nits, can they get back to them quickly?

@kesara
Copy link
Member Author

kesara commented Nov 14, 2024

If someone really wanted the v2 nits, can they get back to them quickly?

No there are no quick links for that. Author tools just shows the output from idnits.

@rjsparks
Copy link
Member

so maybe instead of replacing the idnits2 button, we provide both idnits2 and idnits3 buttons until idnits3 becomes the thing in the submit path?

@cabo
Copy link
Collaborator

cabo commented Nov 16, 2024

Where can I find idnits3?
https://github.com/ietf-tools/idnits does not appear to be in a useful state.

@kesara
Copy link
Member Author

kesara commented Nov 16, 2024

Where can I find idnits3? https://github.com/ietf-tools/idnits does not appear to be in a useful state.

@cabo, You can install with npm 1 or use idnits3 on Auhtor Tools.

Footnotes

  1. https://github.com/ietf-tools/idnits?tab=readme-ov-file#installation

@kesara
Copy link
Member Author

kesara commented Nov 16, 2024

so maybe instead of replacing the idnits2 button, we provide both idnits2 and idnits3 buttons until idnits3 becomes the thing in the submit path?

One of the motivations of this change is to stop unnecessary idnits2 checks because this makes a call to datatracker which seems slow and could affect datatracker performance.

@cabo
Copy link
Collaborator

cabo commented Nov 16, 2024

@cabo, You can install with npm 1 or use idnits3 on Auhtor Tools.

OK, I have that, so "3.0.0-alpha.36" appears to be the most recent revision.
That is not very useful.

https://author-tools.ietf.org/api/idnits3?url=https://www.ietf.org/archive/id/draft-ietf-cbor-cddl-more-control-07.xml
https://mailarchive.ietf.org/arch/msg/last-call/IM2SIo26RzBq0tO0lG5JETyGGNo

@kesara
Copy link
Member Author

kesara commented Nov 17, 2024

@cabo, You can install with npm 1 or use idnits3 on Auhtor Tools.

OK, I have that, so "3.0.0-alpha.36" appears to be the most recent revision. That is not very useful.

https://author-tools.ietf.org/api/idnits3?url=https://www.ietf.org/archive/id/draft-ietf-cbor-cddl-more-control-07.xml https://mailarchive.ietf.org/arch/msg/last-call/IM2SIo26RzBq0tO0lG5JETyGGNo

@cabo, idnits v3 is still in alpha stage of development. But you can report any issues to idnits repository1 so we can correct them early.

Footnotes

  1. https://github.com/ietf-tools/idnits/issues/new

@kesara kesara marked this pull request as draft November 19, 2024 00:03
@kesara kesara changed the title feat: Use idnits3 for post submission nits check feat: Add idnits3 for post submission nits check button Nov 19, 2024
Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.94%. Comparing base (c7f6bde) to head (92d7f0e).
Report is 188 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8169      +/-   ##
==========================================
+ Coverage   88.78%   88.94%   +0.16%     
==========================================
  Files         296      303       +7     
  Lines       41320    41288      -32     
==========================================
+ Hits        36687    36725      +38     
+ Misses       4633     4563      -70     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@kesara kesara marked this pull request as ready for review November 19, 2024 00:58
@rjsparks
Copy link
Member

We need to signal that this is a new, not yet complete, and under development tool. Maybe we can use a tooltip, but there are accessibility concerns with that. Maybe we should put it on its own row with explanatory text? Other ideas?

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.

Update nits check URL
4 participants