Conversation
There's a lot these days! I got most of it from the GitHub docs [1], as well as looking at other projects. I'll probably hold off on landing this until a couple more things land, then land this and tag v0.1.0. [1] https://docs.github.com/en/communities Issue: #35 Test plan: view in github UI Reviewers: steve, jvoll, marksandstrom, mahtab, miguel, adam
| See also: Khan/khan.github.io#17 |
| Nice! Yeah, there are a lot of little bits and pieces, but it does make for a cleaner launch. Thanks! |
jvoll left a comment
There was a problem hiding this comment.
I've never open-sourced anything before but, this seems good to me!
StevenACoffman left a comment
There was a problem hiding this comment.
One other possible badge. It looks like it's impossible for the test badge to ever change though?
README.md Outdated
| [](https://pkg.go.dev/github.com/Khan/genqlient) | ||
| [](https://github.com/Khan/genqlient/actions) | ||
| [](docs/CODE_OF_CONDUCT.md) | ||
| |
There was a problem hiding this comment.
| [](https://goreportcard.com/report/github.com/Khan/genqlient) | |
| The https://goreportcard.com/report/github.com/Khan/genqlient reports some linting nits which it might be worth addressing to give people more confidence.
|
What do you mean? I think it would change if, say, someone merged a PR overriding the failing checks. Or am I missing something that would cause it to stay green even in that case? (Of course we don't intend to do that, and honestly I think all the badges* are kinda unnecessary, but I can see how it provides some value for people to know that we are actually careful about the things we should be careful about.) *other than the pkg.go.dev one, which is a useful icon for a necessary link
I fixed the typo (#100). The rest of the suggestions appear to be:
That doesn't make me think there is significant value to genqlient in suggestions Go Report Card. I can add the badge, though, since I guess an "A" is a "good" grade even if I fully intend to ignore their suggestions going forward. |
(begrudgingly, but anyway)
| Ah great. Thanks! |
I'm not going to really publicize it until the blog post is done, but with all the usual boilerplate set up, it's time to remove this note! (Actually, I meant to do it in #96, but forgot.) Test plan: read it Reviewers: steve, marksandstrom, kevindangoor
## Summary: I'm not going to really publicize it until the blog post is done, but with all the usual boilerplate set up, it's time to remove this note! (Actually, I meant to do it in #96, but forgot.) ## Test plan: read it Author: benjaminjkraft Reviewers: dnerdy, dangoor, StevenACoffman Required Reviewers: Approved By: dnerdy, dangoor Checks: ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint, ✅ Test (1.17), ✅ Test (1.16), ✅ Test (1.15), ✅ Test (1.14), ✅ Lint Pull Request URL: #102
Summary:
There's a lot these days! I got most of it from the GitHub docs [1], as
well as looking at other projects. I'll probably hold off on landing
this until a couple more things land, then land this and tag v0.1.0.
[1] https://docs.github.com/en/communities
Issue: #35
Test plan: view in github UI