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

add sops (mozilla-services/sops) #2603

Closed
wants to merge 1 commit into from
Closed

add sops (mozilla-services/sops) #2603

wants to merge 1 commit into from

Conversation

adrianosela
Copy link
Contributor

SOPS is an encryption at rest -- secrets management solution -- which allows teams to access secrets in encrypted files by having a minimum number of keys as per configuration, for example a personal PGP key and a shared AWS KMS or GCP KMS key.

Make sure that you've checked the boxes below before you submit PR:

  • I have added my package in alphabetical order.
  • I have an appropriate description with correct grammar.
  • I know that this package was not listed before.
  • I have added godoc link to the repo and to my pull request.
  • I have added coverage service link to the repo and to my pull request.
  • I have added goreportcard link to the repo and to my pull request.
  • I have read Contribution guidelines, maintainers note and Quality standard.

Copy link
Owner

@avelino avelino left a comment

Choose a reason for hiding this comment

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

Review all reportcard topics pls

@kirillDanshin
Copy link
Contributor

I believe that due to a delay with the review this project got some new issues with the go lint. They needs to be fixed or at least discussed before merging.

I took a look in the tests and I this that the tests looks reasonable. Maintainers just omitted some functions with something like x := make([]byte, 32); rand.Read(x), so there's no reason to seriously test that, it's like writing tests for the language itself and for the stdlib.

@adrianosela
Copy link
Contributor Author

@kirillDanshin The lint fixes are already in the "development" branch of SOPS. Whenever the team decides its a good time to cut a new release, they will be merged to master

@kirillDanshin
Copy link
Contributor

Actually, as I can see the PRs mentioned here are merged in master

@adrianosela
Copy link
Contributor Author

Screen Shot 2019-09-20 at 12 28 04 PM

@kirillDanshin

@kirillDanshin
Copy link
Contributor

@adrianosela here's your commit in sops master: getsops/sops@4b99fa1

image

Here's the current goreportcard golint:
image

I saw your commits from that PRs in the master before typing in previous message :)

I think what we can do here is just get them a golint in their CI, so the maintainers can monitor golint warnings before merging pull requests, and we can also fix current issues. Unfortunately, I'm not sure if I'll be able to send them a PR in the near future, so if anyone reading this have some spare time, then go ahead – sops is a great project

@adrianosela
Copy link
Contributor Author

@kirillDanshin You are correct, my mistake. @avelino SOPS moves too fast to be squashing lint fixes, and the CI on it doesnt require them. How do we move on from here? I believe given that it is a project maintained by a large org (Mozilla) we should let it in; what do you think?

@avelino
Copy link
Owner

avelino commented Sep 27, 2019

We must presar for quality libraries (I assume this as a premise), not all software/library maintained by large corporations has quality (or thing to be improved).
If it does not meet the minimum quality we are asking for me should not enter, We're not asking much but the least

@avelino
Copy link
Owner

avelino commented Sep 29, 2019

@adrianosela I saw that you are a project contributor, when you have time to leave the project with the minimum quality we ask to open a new PR please

@avelino avelino closed this Sep 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants