Pylons / deform

A Python HTML form library.
Other
416 stars 160 forks source link

Drop py27, pypy, py34 from tox and travis; update coverage #400

Closed stevepiercy closed 4 years ago

stevepiercy commented 4 years ago

Update coverage

However coverage will continue to fail, but not from any changes in this PR. We drifted from 100% coverage at some point in time.

I think this is an improvement over the current situation, and is worth merging. Then we can focus on the missing coverage and broken functional tests on Python 3 only.

See also #397 from which I pulled some useful bits.

stevepiercy commented 4 years ago

Evidently Firefox latest does not work with a simple replacement. It appears that it cannot load a profile. There must be some way to get that profile to load and launch Firefox.

For now I have reverted to Firefox 43. Functional tests are running, but some will fail.

digitalresistor commented 4 years ago

@stevepiercy please undo the change to only run two parts of the tests on Travis...

stevepiercy commented 4 years ago

Lint passes in Travis now.

Locally for just me, when I run tox -e lint, here's the output for check-manifest. I'm not sure how to resolve this.

lint run-test: commands[3] | check-manifest
lists of files in version control and sdist do not match!
missing from sdist:
  deform/static/pickadate/translations/fa_ir.js
suggested MANIFEST.in rules:
  recursive-include deform *.js

That file exists in my checkout. I also tried the suggestion with no effect.

When I run python setup.py sdist, the file shows up in the output and in the sdist:

copying deform/static/pickadate/translations/fa_ir.js -> deform-2.0.9.dev0/deform/static/pickadate/translations

But when I run check-manifest in my virtual environment, I get the same result as running tox -e lint. 😕

sydoluciani commented 4 years ago

deform/static/pickadate/translations/fa_ir.js

Lint passes in Travis now.

Locally for just me, when I run tox -e lint, here's the output for check-manifest. I'm not sure how to resolve this.

lint run-test: commands[3] | check-manifest
lists of files in version control and sdist do not match!
missing from sdist:
  deform/static/pickadate/translations/fa_ir.js
suggested MANIFEST.in rules:
  recursive-include deform *.js

That file exists in my checkout. I also tried the suggestion with no effect.

When I run python setup.py sdist, the file shows up in the output and in the sdist:

copying deform/static/pickadate/translations/fa_ir.js -> deform-2.0.9.dev0/deform/static/pickadate/translations

But when I run check-manifest in my virtual environment, I get the same result as running tox -e lint. 😕

@stevepiercy I am not getting any error, and there is already a line in MANIFEST.in to include all javascripts: recursive-include *.js

However getting warning running 'python setup.py sdist' warning: manifest_maker: MANIFEST.in, line 12: 'recursive-include' expects <dir> <pattern1> <pattern2> ...

That line should change to: recursive-include deform .js or more precisely: recursive-include deform/static .js

It is best to add build environment to tox.ini the same as pyramid: https://github.com/Pylons/pyramid/blob/master/tox.ini#L72

When do you think this release will be available on pypi ?

stevepiercy commented 4 years ago

@sydoluciani I'll take another stab at my local lint. This PR is fine and can be merged as is.

Releasing is another matter. We need to prep it and get a maintainer on board (@mcdonc @ericof @miohtama). We could also add another Pylons Project core contributor to PyPI maintainers. I volunteer as user stevepiercy.

We also need to make a release for deformdemo at the same time. I can start on this this weekend.

stevepiercy commented 4 years ago

I cannot figure out what is the deal with that one file deform/static/pickadate/translations/fa_ir.js on my local environment.

I updated MANIFEST.in and added a build env in tox.ini.

I'll hit up @ericof and @miohtama via other channels to ask if they want to grant me access as a maintainer on Deform to PyPI.

ericof commented 4 years ago

@stevepiercy I already went on Pypi to give you access but I do not have the proper permissions there. I could do a release if needed.

stevepiercy commented 4 years ago

Thanks, @ericof. If you would please review this PR, that would be a good first step. I can prep a release on a new PR, and make sure that everything we want to get into a 3.0.0 release is well documented, tested, etc.

tdamsma commented 4 years ago

Just an outsider comment, I noticed this PR and see that it would also upgrade Bootstrap. Wouldn't that be a major breaking change, effectively turning this into deform3, as this requires every site built with deform forms to upgrade to BS4? Don't think you can mix two bootstrap 3 and 4 on a single page? Don't get me wrong, I very much would like to see this merged, just that this big change is not so apparent from the tile/description of the PR. Keep up the good work, much appreciated!

sydoluciani commented 4 years ago

@tdamsma We discussed the senario of shipping both Bootstrap 3 and Bootstrap 4, but due to lack of time to support both versions, decision made to stay with Bootstrap 4 only. old sites that are already using Bootstrap 3 not going to upgrade to Deform 3, no point to upgrade if they are going to stay with Bootstrap 3, but users like I that is starting up with a new site, definitely going to use Bootstrap 4. currently I am using Bootstrap 4 on my site, with Deform which is using Bootstrap 3. you can always override the css and javascript of master layout template on pages that are loading Deform forms.

@mcdonc @ericof @miohtama If some one can please review the code so we can merge and use the new version. Deform code itself has not changed, only functional tests, and coverage tests has updated to pass the tests. only two of the bootstrap files have replaced to support Bootstrap 4.

Thank you

stevepiercy commented 4 years ago

@tdamsma for developers that will stick with Bootstrap 3, they will need to pin Deform to <3.0 or override assets. We need to document how to do override assets in the change history.

@sydoluciani I now have PyPI release ability thanks to @mcdonc. I have not had time to do the work for a 3.0 release yet due to $WORK, but it is my top FOSS to-do item.

stevepiercy commented 4 years ago

This PR has had plenty of time for review. I'm going to merge it now, and start work on a release soon.