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

[docs] Documents openwisp-utils-qa-checks #47 #65

Merged
merged 1 commit into from
Jan 2, 2020

Conversation

talha-p
Copy link
Contributor

@talha-p talha-p commented Dec 27, 2019

Adds openwisp-utils-qa-checks to the README file and describes its purpose. It also includes checkendline

Fixes #47

@coveralls
Copy link

coveralls commented Dec 27, 2019

Coverage Status

Coverage remained the same at 92.857% when pulling e43eeda on TPath123:qa-docs into 2110cb8 on openwisp:master.

README.rst Outdated Show resolved Hide resolved
Copy link
Member

@atb00ker atb00ker left a comment

Choose a reason for hiding this comment

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

  1. Rebase with master.
  2. master contains openwisp-utils-qa-checks section. Improve on that.
  3. Please document all the options as well. (for all the openwisp-utils-qa-checks as well, like commitcheck, migration name check) You'll have to read some of the code for that as well!

@talha-p
Copy link
Contributor Author

talha-p commented Dec 28, 2019

@atb00ker Ah I see, it must’ve been added after I made my commit (was away for a few days so couldn’t push immediately)
For your final comment, I did state that “all of the above options can be used inside qa-checks”, as they are all documented previously - should I make it more explicit then?

@atb00ker
Copy link
Member

For your final comment, I did state that “all of the above options can be used inside qa-checks”, as they are all documented previously - should I make it more explicit then?

Some options are not documented, please read the code & testcases for all the tests, it should help you find out, when you think you've found all of them, mark for review and I'll double check to let you know! 😄

@talha-p talha-p force-pushed the qa-docs branch 2 times, most recently from b44f619 to c23205d Compare December 29, 2019 20:00
Copy link
Member

@atb00ker atb00ker left a comment

Choose a reason for hiding this comment

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

Okay, it's going good.
Please see the comments below.

README.rst Outdated Show resolved Hide resolved
README.rst Outdated
The following quality assurance checks are ran in openwisp-utils-qa-checks:

* ``checkmigrations`` (Read more below)
* ``checkcommit`` (Read more below)
Copy link
Member

Choose a reason for hiding this comment

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

This line "Simulation of a special unplanned case\n\n#noqa" shows an undocumented change. Please document #noqa case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@atb00ker Could you explain what you mean here? I believe the #noqa case is documented above?

Copy link
Member

Choose a reason for hiding this comment

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

Can you please send the link?

README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
@talha-p talha-p force-pushed the qa-docs branch 3 times, most recently from f756ec8 to 6f09481 Compare January 1, 2020 19:56
This commit adds openwisp-utils-qa-checks to the README file and describes
its purpose. It also includes checkendline

Fixes openwisp#47
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

@atb00ker is this ready to merge?

@nemesifier nemesifier merged commit cdcacf5 into openwisp:master Jan 2, 2020
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.

[docs] Document openwisp-utils-qa-check
4 participants