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

Ordering in the rrule documentation keywords explanation #686

Closed
ridhi1412 opened this issue Apr 16, 2018 · 3 comments · Fixed by #695
Closed

Ordering in the rrule documentation keywords explanation #686

ridhi1412 opened this issue Apr 16, 2018 · 3 comments · Fixed by #695

Comments

@ridhi1412
Copy link
Contributor

There is a difference in the order in which the arguments are listed in the function definition(at the beginning of the documentation), and the order in which they are explained. Specifically, cache is the last argument given when we are defining the function in the documentation, but is the first one to be explained. (http://dateutil.readthedocs.io/en/stable/rrule.html). Another out-of-order keyword is byeaster.

Should we change the ordering in http://dateutil.readthedocs.io/en/stable/_modules/dateutil/rrule.html#rrule to make it consistent?

@pganssle
Copy link
Member

Yes these should have a consistent order. Good catch.

@ridhi1412
Copy link
Contributor Author

So, does this just involve changing the order in the code "http://dateutil.readthedocs.io/en/stable/_modules/dateutil/rrule.html#rrule" ?

So, can i just change the order and submit a PL, without running any tests explicitly from my end?

@pganssle
Copy link
Member

@rmahajan14 For changes to the docs you should run tox -e docs on your end to ensure that the documentation builds correctly.

Not sure what you mean by "changing the order in the code", though. The order of the parameters needs to stay the same, because they are not keyword-only arguments. You'll want to change it in the docstring.

After that, go into docs and run make html, then you can view the documentation under the _build directory to ensure that your changes worked correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants