-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
docs: Update the Overview section of the contributing guide #15674
Conversation
Nice one @marenwestermann , good to make it easier
A couple of things which regularly comes up on the Discord (which you should join btw! check the readme for the link) are:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
All your additions look correct and it flows nicely. I have one comment about updating PRs (merge vs rebase).
So far I have been apprehensive to add much of this information as I want to avoid turning the contributing guide into a "How to use git" tutorial. But then again, I do find myself Googling some of these commands every now and then when setting up a local repo on a new machine, so it might not be a bad idea to have it all in one place.
Fresh eyes on the docs are always very welcome - doubly so if it helps reduce friction for new contributors! ✌️ |
I edited the PR. I
This is a fair point. The question, "How much background knowledge can I expect?" is not an easy one to answer, especially when looking from the diversity and inclusion perspective. I'm happy to remove the section about setting the upstream (lines 57 to 73) because it's already documented in the link about forking a repo (line 52) which I only realised afterwards. |
It's not just a matter of how much background knowledge a person has. It's also that everyone does git differently - IDEs take away the need to type most git commands, and I expect especially newer people to rely on this. So spelling out all the commands may do more harm than good. That's why I would prefer to say something like "commit and push your changes" rather than adding a list of git commands that does this. If you don't know how do do it, it's easy enough to find out. If you already know how to do it, a list of commands you're not familiar with may make you scratch your head. Anyway, I'll give it a bit of thought. Can probably go ahead with most of these changes. |
|
||
```bash | ||
make requirements | ||
make build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realised that one should run make build
, otherwise the polars version is not updated. I saw that in the Makefile you provide several options for compiling and installing. I chose this one because it is the simplest and therefore probably the most beginner-friendy one.
@stinodego did you have think about this? In my opinion when people want to contribute to open source they need to learn how to use the command line anyway so it's useful to also learn the git commands. At the open source community sprints I've been involved in (mostly scikit-learn and pandas) we always taught people to use the command line, including the git commands and to the best of my knowledge we never had any problems with this. However, I also see your point and it's possible that newer people rely on IDEs more and that spelling out commands might confuse people. I don't have any data on this. So let me know what you think. |
9d527c9
to
c2fd1b3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for getting back to this so late @marenwestermann . Ultimately, I am not happy with some of these additions because I feel it adds too much clutter to the contributing guide. I went through the changes and picked the things I liked and got rid of the rest.
I wanted to suggest moving some of these additions to a separate section under contributing, e.g. "Using Git", but then why is this even in the Polars contributing guide? There are plenty of Git guides out there that are just one search away. I know git commands can be confusing even for more experienced developers, but that still doesn't mean this should be in our contributing guide.
I can understand the appeal of having everything in one place, but I prefer to keep our documentation as Polars-specific as possible.
In any case, thanks for your PR and the effort. It can be merged in its current form.
Hi @stinodego. No worries and thank you for taking the time to review this PR. I can understand your point of view and the reason behind it. I'm happy to see that you found some of the additions I made useful. |
I extended the "Contributing to the codebase" section under "Development" to make it easier for people to contribute to the code base. Many career changers in IT belong to an underrepresented group in tech and therefore having a more detailed documentation on how to contribute has the potential to increase the diversity of contributors to Polars. The changes will also make it easier for you to run "Contributing to Polars" community sprints (e.g. at conferences) if you're interested in running these.
In my opinion it would also be helpful to have a section about "Updating the development environment". I wasn't entirely sure how to do it but it seems like people need to run
(Please correct me if I'm wrong.)
I couldn't find an issue on the issue tracker about updating the "Development" documentation so I didn't link an issue here. However, I'm happy to open an issue about it and link this PR if you prefer.
PS: Your documentation on how to contribute to Polars is already pretty good! I enjoyed working through it and everything worked like a charm! :)