modlinltd / django-advanced-filters

Add advanced filtering abilities to Django admin
https://pypi.org/project/django-advanced-filters/
MIT License
771 stars 173 forks source link

A bit of README polishing #54

Closed daonb closed 7 years ago

daonb commented 7 years ago

Thanks for a great README.

Other then the minor edits I've made, there are two things we can improve:

  1. migrations - IMHO, post 1.0 we need to publish migrations
  2. contributing guide - How do I install the dev environment, test and start coding?

regarding 1., If you'd like I can replace the paragraph about migrations with a paragraph about migrating from 0.x

Regarding 2, I'm stuck. python setup.py test crashes. Guess I'm doing something wrong. Here's the log:

actionid: py26-d15
msg: getenv
cmdargs: ['/Users/daonb/Source/matific/django-advanced-filters/.tox/py26-d15/bin/pip', 'install', 'Django>=1.5,<1.6', 'unittest2', '-rtest-reqs.txt']

DEPRECATION: Python 2.6 is no longer supported by the Python core team, please upgrade your Python. A future version of pip will drop support for Python 2.6
Collecting Django<1.6,>=1.5
  Using cached Django-1.5.12-py2.py3-none-any.whl
Collecting unittest2
  Using cached unittest2-1.1.0-py2.py3-none-any.whl
Collecting coveralls==0.5 (from -r test-reqs.txt (line 1))
/Users/daonb/Source/matific/django-advanced-filters/.tox/py26-d15/lib/python2.6/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:318: SNIMissingWarning: An HTTPS request has been made, but the SNI (Subject Name Indication) extension to TLS is not available on this platform. This may cause the server to present an incorrect TLS certificate, which can cause validation failures. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#snimissingwarning.
  SNIMissingWarning
/Users/daonb/Source/matific/django-advanced-filters/.tox/py26-d15/lib/python2.6/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py:122: InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail. You can upgrade to a newer version of Python to solve this. For more information, see https://urllib3.readthedocs.io/en/latest/security.html#insecureplatformwarning.
  InsecurePlatformWarning
  Using cached coveralls-0.5.zip
Collecting factory-boy==2.5.2 (from -r test-reqs.txt (line 2))
  Using cached factory_boy-2.5.2-py2.py3-none-any.whl
Collecting pep8==1.6.2 (from -r test-reqs.txt (line 3))
  Using cached pep8-1.6.2-py2.py3-none-any.whl
Collecting pytest-django==2.9.1 (from -r test-reqs.txt (line 4))
  Using cached pytest_django-2.9.1-py2.py3-none-any.whl
Collecting six>=1.4 (from unittest2)
  Using cached six-1.10.0-py2.py3-none-any.whl
Collecting traceback2 (from unittest2)
  Using cached traceback2-1.4.0-py2.py3-none-any.whl
Collecting argparse (from unittest2)
  Using cached argparse-1.4.0-py2.py3-none-any.whl
Collecting PyYAML>=3.10 (from coveralls==0.5->-r test-reqs.txt (line 1))
  Using cached PyYAML-3.12.tar.gz
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/private/var/folders/0n/0hzyh13d0t71246v74_pqp_r0000gp/T/pip-build-7sm0je/PyYAML/setup.py", line 83, in <module>
        from wheel.bdist_wheel import bdist_wheel
      File "/Users/daonb/Source/matific/django-advanced-filters/.tox/py26-d15/lib/python2.6/site-packages/wheel/bdist_wheel.py", line 407
        ignore=lambda x, y: {'PKG-INFO', 'requires.txt', 'SOURCES.txt',
                                       ^
    SyntaxError: invalid syntax

    ----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /private/var/folders/0n/0hzyh13d0t71246v74_pqp_r0000gp/T/pip-build-7sm0je/PyYAML/
asfaltboy commented 7 years ago

Hi, thanks for the README improvements, these look good and good to merge if you're done.

  1. migrations - IMHO, post 1.0 we need to publish migrations

I can't remember what I meant in that paragraph, since migrations are already published as far as I can tell. Perhaps we should remove this section since it's more confusing than helpful.


  1. contributing guide - How do I install the dev environment, test and start coding?

In essence a contributor would need to do the following:

  1. check out code (i.e git clone)
  2. run tox either directly or via setup.py helper (i.e tox or python setup.py test):

    1. To run the tests for all python/django versions supported, you would need to have them locally installed and on $PATH, for example by using pyenv local.
    2. To run for a specific version of python/django, you may pass the environment argument to select this specific combination. E.g:

      $ tox -e py35-d110

When developing, it is suggested to run the smallest test case possible, and then expand from there to the full suite. For your convenience, the project is configured to run a travis-ci build (full matrix) on every commit and pull-request.

Feel free to commit the above section (or a clearer version) to this PR.

asfaltboy commented 7 years ago

On a side note, we should probably make setup.py test run the tests without any installation of environments or anything, as seen here: https://tox.readthedocs.io/en/latest/example/basic.html#integration-with-setup-py-test-command

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 96.067% when pulling 97ecac44e0a3e1d6dcb8d1b8615e89d30426592b on daonb:better-docs into bf4eb1281b02b530f89b3d103f5b726794a8f964 on modlinltd:develop.

daonb commented 7 years ago

I removed the confusing migrations paragraph, please merge. I open to send a separate PR with the contribution section.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 96.067% when pulling 838a54c260171390eb367e7bd4a7d53023fad19a on daonb:better-docs into bf4eb1281b02b530f89b3d103f5b726794a8f964 on modlinltd:develop.