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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOC] Add hall-of-fame widget to README (Added the Hall-of-fame section) #3716 #6203

Merged
merged 5 commits into from Mar 24, 2024

Conversation

KaustubhUp025
Copy link
Contributor

@KaustubhUp025 KaustubhUp025 commented Mar 24, 2024

Reference Issues/PRs

Add Hall-of-Fame section, fixes #3716

What does this implement/fix? Explain your changes.

The Hall-of-Fame section is added after (How-to-get-involved)

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

No

What should a reviewer concentrate their feedback on?

The code written for keeping track of contributions to the repository.

Did you add any tests for the change?

Not Yet

Any other comments?

PR checklist

For all contributions
  • I've added myself to the list of contributors with any new badges I've earned :-)
    How to: add yourself to the all-contributors file in the sktime root directory (not the CONTRIBUTORS.md). Common badges: code - fixing a bug, or adding code logic. doc - writing or improving documentation or docstrings. bug - reporting or diagnosing a bug (get this plus code if you also fixed the bug in the PR).maintenance - CI, test framework, release.
    See here for full badge reference
  • Optionally, for added estimators: I've added myself and possibly to the maintaners tag - do this if you want to become the owner or maintainer of an estimator you added.
    See here for further details on the algorithm maintainer role.
  • [馃憤 ] The PR title starts with either [ENH], [MNT], [DOC], or [BUG]. [BUG] - bugfix, [MNT] - CI, test framework, [ENH] - adding or improving code, [DOC] - writing or improving documentation or docstrings.
For new estimators
  • I've added the estimator to the API reference - in docs/source/api_reference/taskname.rst, follow the pattern.
  • I've added one or more illustrative usage examples to the docstring, in a pydocstyle compliant Examples section.
  • If the estimator relies on a soft dependency, I've set the python_dependencies tag and ensured
    dependency isolation, see the estimator dependencies guide.

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 24, 2024

Nice! Quick question before review:

why or how is this getting the information from open collective? I am surprised since that's where the svg points to.

Where is that getting it form? The all-contributorsrc, or the repository contributors (which shows only code)?

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 24, 2024

For the emoji, perhaps sth that has closer connotations to thankfulness, achievement or community?

E.g., one of 馃弳 , 馃専 , 馃殌 , 馃殼 , 馃幎 ?
馃帗 has an achievement connotation, but not that much of thanks and community?

fkiraly
fkiraly previously approved these changes Mar 24, 2024
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.

Great addition!

I think this is worth merging and making minor adjustements later.

Most importantly, we should track the discrepancy to the all-contributorsrc and that it does not acknowledge contributions other than code.

@KaustubhUp025
Copy link
Contributor Author

For the question that from where open collective is getting this information, I am finding it the answer for the same but I am in doubt so still searching in the code base. But what I can tell is it derives the data from all-contributorsrc in my opinion.

Also Yes I will give the change to the emoji. @fkiraly

And for the reference for doing this PR, I took reference from the given repo :-
https://github.com/epicmaxco/vuestic-admin/blob/master/README.md?plain=1

@KaustubhUp025
Copy link
Contributor Author

@fkiraly Could you suggest how could we look into the discrepancy involving the all-contributorsrc.

I will make a modification to the emoji as well

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 24, 2024

But what I can tell is it derives the data from all-contributorsrc in my opinion.

I think it doesn't, as far as I understand, it gets it from here:
https://github.com/sktime/sktime/graphs/contributors

Which is the code contribution summary.

The https://github.com/sktime/sktime/blob/main/CONTRIBUTORS.md file gets it from all-contributorsrc, but that seems to be discrepant to the former as well, it lists only 265 instead of 336 contributors.

I would suggest, for now let's open an issue on tracking down where these differences are coming from, but not to invest too much work, or in this PR specifically.

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 24, 2024

@fkiraly Could you suggest how could we look into the discrepancy involving the all-contributorsrc.

I think these are just two separate data sources, and it is very unclear how to reconcile them in the hall-of-fame display. As said, this is probably hard, so let's open an issue where we track this, and get 80% the way there with this PR.

@fkiraly fkiraly added the documentation Documentation & tutorials label Mar 24, 2024
@KaustubhUp025
Copy link
Contributor Author

Thank you so much @fkiraly for your help. So should I do anything else with this PR or move to another issue.

@fkiraly
Copy link
Collaborator

fkiraly commented Mar 24, 2024

I'd suggest you open an issue which describes the remaining todos (e.g., which source is it from, discrepancies), and then I'd consider it complete. Make sure to link this PR for details.

@KaustubhUp025
Copy link
Contributor Author

Thanks @fkiraly, I will add the given points

@fkiraly fkiraly merged commit 09d18f0 into sktime:main Mar 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Documentation & tutorials
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC] Add hall-of-fame widget to README
2 participants