[Freeipa-devel] [freeipa PR#593][opened] WIP: Add make patchcheck for developers

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: WIP: Add make patchcheck for developers
Action: opened

PR body:
"""
Ticket 6604 makes pylint and jsl optional dependencies. The change
is controversal, because some developers prefer that pylint and jsl
should be required unless explicitly disabled.

`make patchcheck` is my answer to address the concerns. It's a superior
solution to `make lint` as pre-commit check. It combines several
additional checks under a single, easy rememberable and convenient make
target:

* build all
* acilint, apiclient, jslint, polint
* make check
* pylint under Python 2 and 3
* subset of unit test suite

https://fedorahosted.org/freeipa/ticket/6604
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: WIP: Add make patchcheck for developers

tiran commented:
"""
Depends on PRs #475, #587, #594
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-286665946

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: WIP: Add make patchcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: WIP: Add make patchcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: WIP: Add make patchcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: WIP: Add make patchcheck for developers
Action: edited

Changed field: title
Original value:
"""
WIP: Add make patchcheck for developers
"""

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make patchcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make patchcheck for developers

tiran commented:
"""
All dependencies have been merged. PR is ready for review.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-287372325

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make patchcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make patchcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make patchcheck for developers

tiran commented:
"""
All dependencies have been merged. PR is ready for review.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-287372325

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make patchcheck for developers
Action: edited

Changed field: body
Original value:
"""
Ticket 6604 makes pylint and jsl optional dependencies. The change
is controversal, because some developers prefer that pylint and jsl
should be required unless explicitly disabled.

`make patchcheck` is my answer to address the concerns. It's a superior
solution to `make lint` as pre-commit check. It combines several
additional checks under a single, easy rememberable and convenient make
target:

* build all
* acilint, apiclient, jslint, polint
* make check
* pylint under Python 2 and 3
* subset of unit test suite

https://fedorahosted.org/freeipa/ticket/6604
"""

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make patchcheck for developers

tiran commented:
"""
Depends on PRs #475, #587, #594
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-286665946

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make patchcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make patchcheck for developers
Action: edited

Changed field: title
Original value:
"""
Add make patchcheck for developers
"""

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make devcheck for developers
Action: edited

Changed field: body
Original value:
"""
Ticket 6604 makes pylint and jsl optional dependencies. The change
is controversal, because some developers prefer that pylint and jsl
should be required unless explicitly disabled.

`make patchcheck` is my answer to address the concerns. It's a superior
solution to `make lint` as pre-commit check. It combines several
additional checks under a single, easy rememberable and convenient make
target:

* build all
* acilint, apiclient, jslint, polint
* make check
* pylint under Python 2 and 3
* subset of unit test suite

https://fedorahosted.org/freeipa/ticket/6604

Depends on

- [X] #475
- [X] #587
- [X] #594
- [ ] #636
- [ ] #670
"""

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make devcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make devcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[stlaz]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

stlaz commented:
"""
The changes to Makefile and configure.ac are just fine. I understand that changes in the `ipapython/session_storage.py` are done elsewhere so once that is pushed, we'll need a rebase.
I don't see the explanation why we're disabling the test in `ipatests/test_ipapython/test_session_storage.py `, that might need a different commit?

"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290651108

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

tiran commented:
"""
```test_session_storage``` is not a unit test or functional test. It is an integration test that depends on a valid Kerberos configuration and session. Do you prefer a separate PR?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290654739

[stlaz]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

stlaz commented:
"""
Whichever is ok with you, I don't mind if it's in the same PR if it is related to the same ticket.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290655653

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make devcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

tiran commented:
"""
I split the changes to session storage tests into a separate commit. The other commit is in #670
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290662222

[stlaz]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

Label: +ack

[stlaz]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

stlaz commented:
"""
Thanks, ACK.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290673932

[stlaz]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

stlaz commented:
"""
@MartinBasti #670 was ACKed already and the commit was originally a part of this.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290678477

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

MartinBasti commented:
"""
@stlaz why is this ACKed when it depends on #670 ?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290678060

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

MartinBasti commented:
"""
Ah right the description hasn't been updated

"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290678887

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make devcheck for developers
Action: edited

Changed field: body
Original value:
"""
Ticket 6604 makes pylint and jsl optional dependencies. The change
is controversal, because some developers prefer that pylint and jsl
should be required unless explicitly disabled.

`make patchcheck` is my answer to address the concerns. It's a superior
solution to `make lint` as pre-commit check. It combines several
additional checks under a single, easy rememberable and convenient make
target:

* build all
* acilint, apiclient, jslint, polint
* make check
* pylint under Python 2 and 3
* subset of unit test suite

https://fedorahosted.org/freeipa/ticket/6604

Depends on

- [X] #475
- [X] #587
- [X] #594
- [x] #636
- [ ] #670
"""

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

MartinBasti commented:
"""
Needs rebase
"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290679169

[tiran]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make devcheck for developers
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

MartinBasti commented:
"""
master:

* e357133fd7b276ccabfe1896ee948f2bb3541d94 Add make devcheck for developers
* 6c092c24b2bfbba0a3f263d88f7a0dbf83f24869 Skip test_session_storage in ipaclient unittest mode


ipa-4-5:

* 89ab24f1fbb58feb603d60503c685ebad41a4237 Add make devcheck for developers
* c80adf6e0d16f807f90479660af22540cd92d774 Skip test_session_storage in ipaclient unittest mode


"""

See the full comment at https://github.com/freeipa/freeipa/pull/593#issuecomment-290691783

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/593
Title: #593: Add make devcheck for developers

Label: +pushed

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/593
Author: tiran
Title: #593: Add make devcheck for developers
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/593/head:pr593
git checkout pr593