Skip to content

Comments

Meta lint openpgp#31

Closed
zugzwang wants to merge 7 commits intoProtonMail:masterfrom
zugzwang:meta-lint-openpgp
Closed

Meta lint openpgp#31
zugzwang wants to merge 7 commits intoProtonMail:masterfrom
zugzwang:meta-lint-openpgp

Conversation

@zugzwang
Copy link
Contributor

@zugzwang zugzwang commented Aug 28, 2019

Closed in favor of 32

Covers openpgp go package only

This branch contains a linted OpenPGP package (crypto/openpgp) with golint, vet, errcheck, deadcode, etc. More precisely, using this plugin and invoking GoMetaLinter.

Notes:

Exported functions, variables, methods, properties, are not corrected in order to preserve the API. Redundant error checking in _test files is ignored as well.

}

if ek.KeyId != keyId || ek.Algo != PubKeyAlgoRSA {
if ek.KeyId != keyID || ek.Algo != PubKeyAlgoRSA {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we have to export KeyId, I think having keyID is kinda inconsistent. I'm not sure that it's worth appeasing the linter for that

p = new(LiteralData)
case packetTypeUserId:
case packetTypeUserID:
p = new(UserId)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similarly here (UserId / UserID). We might just want to keep Id everywhere, linter be damned

}

// ArmorCorrupt is returned if an armor is invalid.
var ArmorCorrupt error = errors.StructuralError("armor invalid")
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know the linter says to add comments to every exported symbol, but some of these are completely self-explanatory even from the name - so then the comment naturally feels kinda redundant.

@zugzwang zugzwang closed this Aug 29, 2019
@zugzwang zugzwang deleted the meta-lint-openpgp branch September 6, 2019 07:59
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.

2 participants