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

Contributing guidelines #758

Merged
merged 11 commits into from
Oct 16, 2023
Merged

Contributing guidelines #758

merged 11 commits into from
Oct 16, 2023

Conversation

wd60622
Copy link
Contributor

@wd60622 wd60622 commented Oct 5, 2023

Adding some information about how to contributing to PyPika after the conversation in this Issue.

I've added a CONTRIBUTING document that will be linked in the docs that build (where do these update by the way?)
Any feedback on content would be helpful!

I just added a few items based on setting up the environment and running tests but this is very flexible.

Does anyone know how to have a checklist added to a new PR via a GitHub action? Maybe a checklist or a link to this documentation would be reminder if the automation is easy to setup

Overall, I hope this helps standardize some of the steps while contributing.

@wd60622 wd60622 requested a review from a team as a code owner October 5, 2023 15:38
@wd60622 wd60622 mentioned this pull request Oct 6, 2023
Copy link

@AzisK AzisK left a comment

Choose a reason for hiding this comment

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

Nice work, thank you!

I have one question. docs/index.rst is not displayed well as an rst file via Github platform but I suppose it looks well when opening in the browser?

@coveralls
Copy link

coveralls commented Oct 6, 2023

Coverage Status

coverage: 98.387%. remained the same when pulling 179aa95 on wd60622:contributing into ded17eb on kayak:master.

@wd60622
Copy link
Contributor Author

wd60622 commented Oct 6, 2023

Nice work, thank you!

I have one questions. docs/index.rst is not displayed well as an rst file via Github platform but I suppose it looks well when opening in the browser?

Correct. Here is the preview from local build. I'm not a sphinx expert with references other pages but it seems to be working
Screenshot 2023-10-06 at 18 19 47

Screenshot 2023-10-06 at 18 19 55

@wd60622
Copy link
Contributor Author

wd60622 commented Oct 6, 2023

Is there a build script that runs upon merge to main?

@AzisK
Copy link

AzisK commented Oct 10, 2023

I believe that everything that runs is described here in the workflows https://github.com/kayak/pypika/tree/master/.github/workflows. Only a linting process and a testing process in various Python versions is done

@wd60622
Copy link
Contributor Author

wd60622 commented Oct 10, 2023

I believe that everything that runs is described here in the workflows https://github.com/kayak/pypika/tree/master/.github/workflows. Only a linting process and a testing process in various Python versions is done

Thats what I made of it too.

Are the docs build manual then?

@AzisK
Copy link

AzisK commented Oct 10, 2023

It seems to be so currently

@wd60622
Copy link
Contributor Author

wd60622 commented Oct 11, 2023

It seems to be so currently

Confirmed here
https://readthedocs.org/projects/pypika/builds/

@wd60622
Copy link
Contributor Author

wd60622 commented Oct 12, 2023

I've added a bit more to the CONTRIBUTING.rst as well as added support for numpy style docstrings (raised by me)

I have successful builds locally but with handful of warnings:

WARNING: toctree contains reference to nonexisting document 'api/pypika.terms.WindowFrameAnalyticFunction.wrap_constant'

@wd60622
Copy link
Contributor Author

wd60622 commented Oct 12, 2023

Ended up adding the missing modules (but didn't explore clickhouse) and checking an example of numpydoc style with in the Query class

@AzisK
Copy link

AzisK commented Oct 16, 2023

Looks good

@AzisK AzisK merged commit 627b60a into kayak:master Oct 16, 2023
5 checks passed
Ga68 added a commit to Ga68/pypika that referenced this pull request Oct 16, 2023
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.

3 participants