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

fix: Use more robust filename/rev matching for meeting materials #5192

Merged
merged 7 commits into from
Feb 24, 2023

Conversation

jennifer-richards
Copy link
Member

Fixes #4731.

Note that this does away with a series of moderately complex regex checks in the revision matching. I don't know the motivation behind those checks, but it seems to me that the get_related_meeting() check should be adequate to distinguish whether a document is suitable to be returned by this method.

Copy link
Member

@rjsparks rjsparks left a comment

Choose a reason for hiding this comment

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

There's also been a discussion about disallowing new document names that end with -nn, and I'd be completely fine forcing that change. (Though it would irritate chairs that like to name their slides after a particular version of a draft, but it won't take long for them to adjust).

if doc is not None and doc.get_related_meeting() == meeting:
return doc, None
# try parsing a rev number
name, rev = name.rsplit('-', 1)
Copy link
Member

Choose a reason for hiding this comment

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

This isn't sufficient. There are drafts out there with names that are prefixes of other names. You need to make sure the thing after the last - looks like two digits before trying to look up what came before.

@codecov
Copy link

codecov bot commented Feb 23, 2023

Codecov Report

Merging #5192 (ca16bde) into main (ce374ac) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5192      +/-   ##
==========================================
+ Coverage   88.54%   88.58%   +0.03%     
==========================================
  Files         295      295              
  Lines       40110    40111       +1     
==========================================
+ Hits        35516    35531      +15     
+ Misses       4594     4580      -14     
Impacted Files Coverage Δ
ietf/meeting/views.py 91.12% <100.00%> (+0.27%) ⬆️
ietf/utils/fields.py 91.75% <100.00%> (ø)
ietf/nomcom/utils.py 91.30% <0.00%> (-0.25%) ⬇️
ietf/doc/models.py 89.01% <0.00%> (+0.85%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rjsparks rjsparks merged commit 91be593 into ietf-tools:main Feb 24, 2023
@jennifer-richards jennifer-richards deleted the jennifer/slide-numbers branch February 24, 2023 19:18
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

slide names ending with numbers are breaking
2 participants