Pylons / deform

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

Fix grammar. Prep for 3.0 release. #407

Closed stevepiercy closed 4 years ago

stevepiercy commented 4 years ago

@sydoluciani I just realized, from your comment that you said that gettext can be deleted, because it should be already installed with the system. It looks like macOS does not include it by default, so I'll leave it in. There also might be some Linuxes that don't have it in their distribution.

Also should I entirely remove the section on Xvfb? It's not relevant to macOS, and I think that it is not required with Firefox and geckodriver on Linux.

Finally if you have any spare time to review this version of contributing.md, I would greatly appreciate it. Thanks for your help!

sydoluciani commented 4 years ago

You are right, we should keep the gettext installation, and yes, we can remove the Xvfb section.

Developers testing locally is best to use FireFox and Xwindows to view the progress of tests and see the error if there is any. Please remove both Xvfb installation and the part that it says: Functional tests behave differently depending on whether you are looking at the browser window or using Xvfb.

At the end we got the same results either using Xvfb or Xwindows.

Please add another line below:

export PATH=~/projects/deform/:$PATH

export WEBDRIVER=/Full_Path_To_geckodriver_file_not_the_directory

The WEBDRIVER environment is going to point to the file itself instead of the directory that is extracted as far as I remember and needed to be set before running the tox or nosetest otherwise firefox or chorm won't start with error message driver not found.

If developers use tox to run tests, then below lines of code are not required, since tox starts pserve, we only need to run pserve, if running the nosetests directly outside of tox. I think we can remove this section. but it is nice to add a section to the end of the file, that we accept contribution to document and test against chrome or chromium. the only different is downloading chrome or chromium instead of Firefox, then download the appropriate Selenium driver for chrome and point the WEBDRIVER environment variable to chromium driver instead of geckodriver. geckodriver is Selenium driver for FireFox but Selenium driver for Chrome or Chromium seems to be chrome or chromium respectively.

To run/edit/fix functional tests. :note: This section has not worked for years, but could be improved for Firefox.

source .tox/functional3/bin/activate cd deformdemo # Checked out by tox functional3 pserve demo.ini # Start web server

The rest look good.

On Wed, 17 Jun 2020 at 21:23, Steve Piercy notifications@github.com wrote:

@sydoluciani https://github.com/sydoluciani I just realized, from your comment https://github.com/Pylons/deform/pull/402/files#r426248136 that you said that gettext can be deleted, because it should be already installed with the system. It looks like macOS does not include it by default, so I'll leave it in. There also might be some Linuxes that don't have it in their distribution.

Also should I entirely remove the section on Xvfb? It's not relevant to macOS, and I think that it is not required with Firefox and geckodriver on Linux.

Finally if you have any spare time to review this version of contributing.md, I would greatly appreciate it. Thanks for your help!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Pylons/deform/pull/407#issuecomment-645729755, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOLSNHK4A6HTAFO5NZMPOXDRXF3C3ANCNFSM4OBENAGQ .

sydoluciani commented 4 years ago

We can remove this section:

To run/edit/fix functional tests. :note: This section has not worked for years, but could be improved for Firefox.

source .tox/functional3/bin/activate cd deformdemo # Checked out by tox functional3 pserve demo.ini # Start web server

Run functional test suite using Chrome

WEBDRIVER="chrome" nosetests -x

Run functional test suite using Chrome, stop on pdb on exception

WEBDRIVER="chrome" nosetests -x --pdb

Run one functional test case using Chrome

WEBDRIVER="chrome" nosetests -x deformdemo.test:SequenceOfDateInputs

Or we can move it to the end of the file, since WEBDRIVER is pointing to chrome driver, and we don't need to run pserve if running tox.

On Thu, 18 Jun 2020 at 01:53, Sydo Luciani sydo.luciani@gmail.com wrote:

You are right, we should keep the gettext installation, and yes, we can remove the Xvfb section.

Developers testing locally is best to use FireFox and Xwindows to view the progress of tests and see the error if there is any. Please remove both Xvfb installation and the part that it says: Functional tests behave differently depending on whether you are looking at the browser window or using Xvfb.

At the end we got the same results either using Xvfb or Xwindows.

Please add another line below:

export PATH=~/projects/deform/:$PATH

export WEBDRIVER=/Full_Path_To_geckodriver_file_not_the_directory

The WEBDRIVER environment is going to point to the file itself instead of the directory that is extracted as far as I remember and needed to be set before running the tox or nosetest otherwise firefox or chorm won't start with error message driver not found.

If developers use tox to run tests, then below lines of code are not required, since tox starts pserve, we only need to run pserve, if running the nosetests directly outside of tox. I think we can remove this section. but it is nice to add a section to the end of the file, that we accept contribution to document and test against chrome or chromium. the only different is downloading chrome or chromium instead of Firefox, then download the appropriate Selenium driver for chrome and point the WEBDRIVER environment variable to chromium driver instead of geckodriver. geckodriver is Selenium driver for FireFox but Selenium driver for Chrome or Chromium seems to be chrome or chromium respectively.

To run/edit/fix functional tests. :note: This section has not worked for years, but could be improved for Firefox.

source .tox/functional3/bin/activate cd deformdemo # Checked out by tox functional3 pserve demo.ini # Start web server

The rest look good.

On Wed, 17 Jun 2020 at 21:23, Steve Piercy notifications@github.com wrote:

@sydoluciani https://github.com/sydoluciani I just realized, from your comment https://github.com/Pylons/deform/pull/402/files#r426248136 that you said that gettext can be deleted, because it should be already installed with the system. It looks like macOS does not include it by default, so I'll leave it in. There also might be some Linuxes that don't have it in their distribution.

Also should I entirely remove the section on Xvfb? It's not relevant to macOS, and I think that it is not required with Firefox and geckodriver on Linux.

Finally if you have any spare time to review this version of contributing.md, I would greatly appreciate it. Thanks for your help!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Pylons/deform/pull/407#issuecomment-645729755, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOLSNHK4A6HTAFO5NZMPOXDRXF3C3ANCNFSM4OBENAGQ .

stevepiercy commented 4 years ago

we can remove the Xvfb section.

Done!

Please add another line below:

export PATH=~/projects/deform/:$PATH export
WEBDRIVER=/Full_Path_To_geckodriver_file_not_the_directory

Done!

If developers use tox to run tests, then below lines of code are not required, since tox starts pserve, we only need to run pserve, if running the nosetests directly outside of tox. I think we can remove this section.

We use tox in most Pylons Projects projects and support it. If a developer decides to use nosetest (or eventually pytest) instead, they probably know what they are doing and don't need support of how to do so.

Please let me know if I addressed this point. I had a hard time parsing the meaning of your reply because it was one big lump of text.

but it is nice to add a section to the end of the file, that we accept contribution to document and test against chrome or chromium. the only different is downloading chrome or chromium instead of Firefox, then download the appropriate Selenium driver for chrome and point the WEBDRIVER environment variable to chromium driver instead of geckodriver. geckodriver is Selenium driver for FireFox but Selenium driver for Chrome or Chromium seems to be chrome or chromium respectively. To run/edit/fix functional tests. :note: This section has not worked for years, but could be improved for Firefox. source .tox/functional3/bin/activate cd deformdemo # Checked out by tox functional3 pserve demo.ini # Start web server The rest look good.

I think I cleaned this up.

We can remove this section:

To run/edit/fix functional tests. :note: This section has not worked for years, but could be improved for Firefox.

Yeah, that's garbage. Deleted. Forward!

Would you please check once more, and provide a review?

BTW, why don't we have you as a Contributor in Deform? You put a lot of thought and effort into contributions, and your efforts are appreciated. Would you like me to grant you access?

sydoluciani commented 4 years ago

It looks good, just need couple of tweaks. I misspelled the chrome in this line: otherwise Firefox or chorm will not start and will return an error message of "driver not found".

Extracting Firefox(tar -xjf firefox-latest.tar.bz2) creates a new directory (firefox), and we should add this directory to path, so this line: export PATH=~/projects/deform/:$PATH will be: export PATH=~/projects/deform/firefox:$PATH

I liked the way we worked on this upgrade(team work), so no need for access. I will submit a pull request if I see any issue I can work on. Thank you :)

I like Pyramid and Deform and spent some time to learn them, in fact I am using them both in my personal projects, however with emerging single page applications like React, and moving from REST to GraphQL, I am not sure where Deform will be fitting in my future projects. considering Pyramid is a micro framework(a very good python micro framework), it is a perfect choice to be backend for React, or just being a GraphQL server. There is already a library(named Graphene) that provides Graphql server based on Pyramid, but there is a need for documentation and skelton to make it easier to start with.

I think a project to have Pyramid, GraghQL and jwt and a project to have combination of Pyramid, React and webpack are viable projects that boost Pyramid based application development.

stevepiercy commented 4 years ago

@sydoluciani thank you! I made those two corrections in my latest commit https://github.com/Pylons/deform/pull/407/commits/235dc70ddd912e6a56e0b57d1881c82df68a25cd

As far as the future of Deform, it will always serve well as a server-side form generator.

Until webob-graphql matures, you might be interested in pyramid_openapi3.

sydoluciani commented 4 years ago

Thank you, like pyramid_openapi3, but for GraphQL and React. If Pylon starts any such project, I will spend couple of hours in weekends to contribute. Even if it starts with documentation of how to put together the React and Pyramid project.

Deform is great and at some point I am going to spend some time to provide an example of horizontal form in deformdemo or document it in Deform documentation. There were attempts but some how it stopped, #214 , #187 We should be able to override the form.pt and mapping-item.pt as it is explained in #214 and create a form with fields aligned horizontally.

For now we can close this issue #355 :)

stevepiercy commented 4 years ago

See #282 for work on both horizontal and inline form layouts. I added it to the 3.0 milestone.

sydoluciani commented 4 years ago

@stevepiercy Let's complete the upgrade first. we pick the horizontal and inline form layout some time later.

stevepiercy commented 4 years ago

@sydoluciani you're right. I moved it to Deform 3.1.0 milestone as a new feature.

sydoluciani commented 4 years ago

@stevepiercy Thank you. I will pick it up after the upgrade, probably in a month. by then we receive enough feedback. thanks very much for all your help and guidance.