Skip to content

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Jan 22, 2020

No description provided.

@sleevi
Copy link
Contributor

sleevi commented Jan 22, 2020

Is this is necessary, given

?

Not an objection, just trying to understand the relationship between things and the style for the lints.

(Sorry, that was supposed to be a question, not a statement)

@jsha
Copy link
Contributor Author

jsha commented Jan 22, 2020

This may just be a matter of me being insufficiently familiar with the style for these lints, but I didn't think to look at the bottom of the file. My workflow was:

  • Read the clause in RFC 5280
  • Look for a matching lint in zlint
  • Check that the lint matches what I expect
  • Compare behavior against quoted text
  • Get confused that what I read in the quoted text doesn't match what I just read in RFC 5280.

So, submitted this as a possible clarification for other folks in the same position, but I'm also fine with it being closed as not following common style.

A broader style fix to address the same confusion might be to move all the func init's to the top of each lint, so the Citation field is closer to the text being cited. Or possible even make the citation text part of the lint.Lint object.

@sleevi
Copy link
Contributor

sleevi commented Jan 22, 2020

Yeah, I'm not sure the style either, which is why I meant to ask the question than the statement. I didn't notice it until expanding to view how it was implemented, and then saw it already referenced 6818.

Definitely, moving init() to be the start would serve a lot of self-documentation for those of us who read top-down :) I filed #371 as my poor attempt to explain your great idea :)

@zakird
Copy link
Member

zakird commented Jan 22, 2020

I agree makes sense to move, but shouldn't get in the way of merging this correction.

@zakird zakird merged commit 77026f6 into zmap:master Jan 22, 2020
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.

4 participants