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

[DOC] Update CONTRIBUTORS.md #3781

Merged
merged 1 commit into from Nov 17, 2022
Merged

[DOC] Update CONTRIBUTORS.md #3781

merged 1 commit into from Nov 17, 2022

Conversation

achieveordie
Copy link
Collaborator

Reference Issues/PRs

See #2905 for reference.

What does this implement/fix? Explain your changes.

Locally ran all-contributors-cli via npm to update CONTRIBUTORS.md to include new changes. The last update was two months ago so I thought it'll be great to have it updated.

Does your contribution introduce a new dependency? If yes, which one?

None

What should a reviewer concentrate their feedback on?

Any other comments?

PR checklist

For all contributions
  • I've added myself to the list of contributors.
  • Optionally, I've updated sktime's CODEOWNERS to receive notifications about future changes to these files.
  • I've added unit tests and made sure they pass locally.
  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG] indicating whether the PR topic is related to enhancement, maintenance, documentation, or bug.

Copy link
Collaborator

@fkiraly fkiraly left a comment

Choose a reason for hiding this comment

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

Nice!

In total it seems to have added three lines though, can that be correct?

Also, I was wondering whether this can be automated, say, as part of the release workflow?

@achieveordie
Copy link
Collaborator Author

In total it seems to have added three lines though, can that be correct?

Yes, it's correct since the number of contributors has gone up from 176 before this PR to 179 in this PR with one line per new contributor.

Also, I was wondering whether this can be automated

That's what I've been considering too. I made a comment in the referenced issue that we could automate this via a GitHub bot, but I suppose GitHub doesn't provide native support to bots (they'd have to be deployed somewhere else and integrated to GH). So we could maybe try to add an additional step in the workflow. But since this involves adding and managing node packages I'm not sure if that's the best course of action.

What are your opinions?

@fkiraly
Copy link
Collaborator

fkiraly commented Nov 14, 2022

What are your opinions?

Well, if I had a good solution in mind, I would have posted it in #2905 or already implemented it - whatever works and isn't too complex to maintain and implement.

@achieveordie
Copy link
Collaborator Author

In that case, we could find a viable solution in by discussing the possible solutions in #2905 - I gave one of the possible solutions, maybe someone over there knows a better way to do it.

@lmmentel
Copy link
Contributor

Nice work @achieveordie. Another possibility for automating would be to add a job to GitHub actions to run the command you ran locally on each PR merge.

@achieveordie
Copy link
Collaborator Author

@lmmentel That's the only idea that can be implemented immediately since other solutions (GitHub App / Bot) would require a third-party integration which I understand sktime wouldn't want to pursue due to additional governance and cost (not to mention it needs to be set up by a core developer). So I'm going to proceed with the GH Actions.

Initially, my problem with this solution was how often this job would be repeatedly called since .allcontributorsrc rarely changes, but I suppose GH Actions must have an answer to it as well.

(Also, I think core developers could give GH Apps / Bots a thought since a lot of maintenance work could then be automated enabling maintainers to focus their time and energy in a better way)

@fkiraly fkiraly merged commit 9780358 into sktime:main Nov 17, 2022
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.

None yet

3 participants