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

Added pydov to package list, and the group of DOV-Vlaanderen as packa… #58

Merged
merged 2 commits into from
Mar 2, 2021

Conversation

pjhaest
Copy link
Contributor

@pjhaest pjhaest commented Feb 19, 2021

Thanks for the good review and the acceptance of pydov (see software review)!
We already included the badge on our GitHub and hereby provide the details of the package and the maintainers.
If we missed something, let us know and we'll make the necessary changes.

Copy link
Contributor

@NickleDave NickleDave left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@NickleDave
Copy link
Contributor

Thank you @pjhaest !!!
Excited to officially add pydov to the package list and have DOV-Vlaanderen as a maintainer.

Just checking:
did you @Roel and @stijnvanhoey want to add yourselves to the contributor list as well?
I guess within PyOpenSci you were the ones that led pydov through the review process.

I feel like we'd want to keep track of the people that were directly involved in making PyOpenSci a community, although maybe you had discussions about this and felt like it was best to credit your whole group as maintainers.
@lwasser please weigh in with what you feel is best of course.

I don't mean to make any more work for anybody, but just thought I should bring that up before I click merge.

@Roel
Copy link

Roel commented Feb 19, 2021

We discussed this but were a little unsure on how to include all our contributers so went for DOV-Vlaanderen instead. Looking more closely now, I see that we maybe can make the distinction between maintainers and contributers and have our three names instead.

I think for us it doesn't really matter, but if you prefer our names we can shurely adapt the PR accordingly.

@NickleDave
Copy link
Contributor

I see, thank you

not trying to complicate things but I think per our spec, you would all have the type "PackageMaintainer". Right now this is defined as "someone who has submitted a package that successfully went through review".

Even though some people added themselves as contributor, I think contributor is a more general term that we just inherit from the all-contributors spec. Probably we should fix that / make it clear but that's an issue for another day.

so, if you're okay with it, I propose:

  • keep DOV-Vlaanderen as PackageMaintainer so the whole group gets credit
  • and the three of you add yourselves individually as PackageMaintainer also

I will make the executive decision this is ok for now and if @lwasser feels strongly otherwise (I don't think she will) then I can fix with another PR later

@pjhaest
Copy link
Contributor Author

pjhaest commented Feb 19, 2021

@NickleDave , thanks for the quick notice! Indeed, it was not entirely clear to us what would be the preferred reference, therefore we picked the entire group. @Roel , @stijnvanhoey, if it's ok with you, I can change the PR to include you as individual PackageMaintainers?

@stijnvanhoey
Copy link

@pjhaest all fine for me. Thanks for fixing this during the sprint! I totally missed this to-do on the pyopensci process.

@lwasser
Copy link
Member

lwasser commented Mar 2, 2021

thank you all for this pr!! @stijnvanhoey @NickleDave @pjhaest i'm happy to merge or @NickleDave would you like to merge?

@NickleDave NickleDave merged commit fec9c1a into pyOpenSci:master Mar 2, 2021
@NickleDave
Copy link
Contributor

oh whoops didn't see that @pjhaest made the changes

merged now!

excellent, you are all officially contributors!!! thank you so much.
The review went well and we'll be happy to tell people about pydov

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.

5 participants