Topics

Extra checks for Pull Requests

Reef Turner
 

Hi everyone,

 

We have recently introduced new checks for Pull Requests on GitHub.

 

Code contributors regularly have to deal with ill-defined and inconsistently enforced code style requirements. Code reviewers spend much of their time reporting minor issues, time that would be better spent looking for architectural problems / product issues / logic errors.

 

To improve this situation, in future PR builds the Flake8 linter will be run checking any new code that will be introduced with a PR. This can also be run with SCons using `scons lint base=origin/master`. Please see the `tests/lint/readme.md` file for more information.

 

As part of a PR build, the Flake8 linter will be run checking any new code that will be introduced with a PR. When Flake8 reports the does not comply with it’s configuration, a message from Appveyor will be added as a comment to the Pull Request, and the build will fail. Please note, that in this case the artifacts (PR build executable) will still be available if those steps were successful.

 

We have included an extension with the linter setup to allow tabs instead of spaces, and my find it necessary to disable other warnings, or introduce workarounds. However, in general we would like to stick as close to the default settings as possible.

 

Thanks for all your contributions!

--

Reef Turner
Software Developer 

 

www.nvaccess.org

Facebook: https://www.facebook.com/NVAccess 
Twitter: @NVAccess 

 

zvonimir stanečić, 9a5dsz
 

Hi Reef,

To hijack the thread…

Why it’s not possible to download the latest alpha?

Somewhere we have 404 errors

 

From: nvda-devel@groups.io <nvda-devel@groups.io> On Behalf Of Reef Turner
Sent: Thursday, August 1, 2019 8:59 PM
To: nvda-devel@groups.io
Subject: [nvda-devel] Extra checks for Pull Requests

 

Hi everyone,

 

We have recently introduced new checks for Pull Requests on GitHub.

 

Code contributors regularly have to deal with ill-defined and inconsistently enforced code style requirements. Code reviewers spend much of their time reporting minor issues, time that would be better spent looking for architectural problems / product issues / logic errors.

 

To improve this situation, in future PR builds the Flake8 linter will be run checking any new code that will be introduced with a PR. This can also be run with SCons using `scons lint base=origin/master`. Please see the `tests/lint/readme.md` file for more information.

 

As part of a PR build, the Flake8 linter will be run checking any new code that will be introduced with a PR. When Flake8 reports the does not comply with it’s configuration, a message from Appveyor will be added as a comment to the Pull Request, and the build will fail. Please note, that in this case the artifacts (PR build executable) will still be available if those steps were successful.

 

We have included an extension with the linter setup to allow tabs instead of spaces, and my find it necessary to disable other warnings, or introduce workarounds. However, in general we would like to stick as close to the default settings as possible.

 

Thanks for all your contributions!

--

Reef Turner
Software Developer 

 

www.nvaccess.org

Facebook: https://www.facebook.com/NVAccess 
Twitter: @NVAccess 

 

Brian's Mail list account
 

Yes I get a nasty dong when it tries to auto update. Probably a broken link somewhere.
Brian

bglists@...
Sent via blueyonder.
Please address personal E-mail to:-
briang1@..., putting 'Brian Gaff'
in the display name field.
Newsgroup monitored: alt.comp.blind-users

----- Original Message -----
From: "zvonimir stanečić, 9a5dsz" <zvonimirek222@...>
To: <nvda-devel@groups.io>
Sent: Thursday, August 01, 2019 8:02 PM
Subject: Re: [nvda-devel] Extra checks for Pull Requests


Hi Reef,

To hijack the thread…

Why it’s not possible to download the latest alpha?

Somewhere we have 404 errors



From: nvda-devel@groups.io <nvda-devel@groups.io> On Behalf Of Reef Turner
Sent: Thursday, August 1, 2019 8:59 PM
To: nvda-devel@groups.io
Subject: [nvda-devel] Extra checks for Pull Requests



Hi everyone,



We have recently introduced new checks for Pull Requests on GitHub <https://github.com/nvaccess/nvda/pull/9958> .



Code contributors regularly have to deal with ill-defined and inconsistently enforced code style requirements. Code reviewers spend much of their time reporting minor issues, time that would be better spent looking for architectural problems / product issues / logic errors.



To improve this situation, in future PR builds the Flake8 linter will be run checking any new code that will be introduced with a PR. This can also be run with SCons using `scons lint base=origin/master`. Please see the `tests/lint/readme.md` file for more information.



As part of a PR build, the Flake8 linter will be run checking any new code that will be introduced with a PR. When Flake8 reports the does not comply with it’s configuration, a message from Appveyor will be added as a comment to the Pull Request, and the build will fail. Please note, that in this case the artifacts (PR build executable) will still be available if those steps were successful.



We have included an extension with the linter setup to allow tabs instead of spaces, and my find it necessary to disable other warnings, or introduce workarounds. However, in general we would like to stick as close to the default settings as possible.



Thanks for all your contributions!

--

Reef Turner
Software Developer



<https://www.nvaccess.org/> www.nvaccess.org

Facebook: https://www.facebook.com/NVAccess
Twitter:@NVAccess
<https://docs.google.com/uc?export=download&id=1ewvP2k8vO2yVx8f7qN46MLkUNt6pt8f_&revid=0B9qtAUOcqzA6dW5BQW55ZEp5UUptWjVYaXF5bnV4VC9qSGVRPQ>

Reef Turner
 

Thanks for the heads up. This is due to the changes I made to how appveyor uploads files. I’ll push a fix for this in the morning. For now, if you want the lastest alpha, you will have to download it directly from the appveyor artifacts page.

 

Here is a direct link for now. Until this is fixed you can do the following:

 

  1. Go to our snapshots page
  2. Copy the link  URL for the build, eg ‘alpha-18266,1af87759’ not ‘download’
  3. Paste in the address bar and add ‘/artifacts’ and press enter
  4. Find the link for ‘nvda_snapshot_alpha-18266,1af87759.exe’ and press enter.

 

From: zvonimir stanečić, 9a5dsz
Sent: Thursday, 1 August 2019 9:03 PM
To: nvda-devel@groups.io
Subject: Re: [nvda-devel] Extra checks for Pull Requests

 

Hi Reef,

To hijack the thread…

Why it’s not possible to download the latest alpha?

Somewhere we have 404 errors

 

From: nvda-devel@groups.io <nvda-devel@groups.io> On Behalf Of Reef Turner
Sent: Thursday, August 1, 2019 8:59 PM
To: nvda-devel@groups.io
Subject: [nvda-devel] Extra checks for Pull Requests

 

Hi everyone,

 

We have recently introduced new checks for Pull Requests on GitHub.

 

Code contributors regularly have to deal with ill-defined and inconsistently enforced code style requirements. Code reviewers spend much of their time reporting minor issues, time that would be better spent looking for architectural problems / product issues / logic errors.

 

To improve this situation, in future PR builds the Flake8 linter will be run checking any new code that will be introduced with a PR. This can also be run with SCons using `scons lint base=origin/master`. Please see the `tests/lint/readme.md` file for more information.

 

As part of a PR build, the Flake8 linter will be run checking any new code that will be introduced with a PR. When Flake8 reports the does not comply with it’s configuration, a message from Appveyor will be added as a comment to the Pull Request, and the build will fail. Please note, that in this case the artifacts (PR build executable) will still be available if those steps were successful.

 

We have included an extension with the linter setup to allow tabs instead of spaces, and my find it necessary to disable other warnings, or introduce workarounds. However, in general we would like to stick as close to the default settings as possible.

 

Thanks for all your contributions!

--

Reef Turner
Software Developer 

 

www.nvaccess.org

Facebook: https://www.facebook.com/NVAccess 
Twitter: @NVAccess 

 

 

Patrick ZAJDA
 

Hello Reef,

First of all, nice to see this great news.

It looks like there is something strange, or globalCommand.py must be reviewed :)

The "script" decorator make these new tests failed with this message:

"

E121 continuation line under-indented for hanging indent"

I wanted to test with my pull request as it is not a bug I though it could be good to see what happens.

I might have done something bad, even if I read and the other script with the decorator has the same style as the code I made.

What should I do to pass these tests?

Thanks,

Patrick

Le 01/08/2019 à 20:58, Reef Turner a écrit :

Hi everyone,

 

We have recently introduced new checks for Pull Requests on GitHub.

 

Code contributors regularly have to deal with ill-defined and inconsistently enforced code style requirements. Code reviewers spend much of their time reporting minor issues, time that would be better spent looking for architectural problems / product issues / logic errors.

 

To improve this situation, in future PR builds the Flake8 linter will be run checking any new code that will be introduced with a PR. This can also be run with SCons using `scons lint base=origin/master`. Please see the `tests/lint/readme.md` file for more information.

 

As part of a PR build, the Flake8 linter will be run checking any new code that will be introduced with a PR. When Flake8 reports the does not comply with it’s configuration, a message from Appveyor will be added as a comment to the Pull Request, and the build will fail. Please note, that in this case the artifacts (PR build executable) will still be available if those steps were successful.

 

We have included an extension with the linter setup to allow tabs instead of spaces, and my find it necessary to disable other warnings, or introduce workarounds. However, in general we would like to stick as close to the default settings as possible.

 

Thanks for all your contributions!

--

Reef Turner Software Developer 

 

www.nvaccess.org

Facebook: https://www.facebook.com/NVAccess  Twitter: @NVAccess 

 

--
Patrick ZAJDA
Certification NVDA 2019

Brian's Mail list account
 

Hmm, well I think I'll wait for a fix. grin.
Brian

bglists@...
Sent via blueyonder.
Please address personal E-mail to:-
briang1@..., putting 'Brian Gaff'
in the display name field.
Newsgroup monitored: alt.comp.blind-users

----- Original Message -----
From: "Reef Turner" <reef@...>
To: <nvda-devel@groups.io>
Sent: Thursday, August 01, 2019 9:28 PM
Subject: Re: [nvda-devel] Extra checks for Pull Requests


Thanks for the heads up. This is due to the changes I made to how appveyor uploads files. I’ll push a fix for this in the morning. For now, if you want the lastest alpha, you will have to download it directly from the appveyor artifacts page.

Here is a direct link for now. Until this is fixed you can do the following:

1. Go to our snapshots page
2. Copy the link URL for the build, eg ‘alpha-18266,1af87759’ not ‘download’
3. Paste in the address bar and add ‘/artifacts’ and press enter
4. Find the link for ‘nvda_snapshot_alpha-18266,1af87759.exe’ and press enter.

From: zvonimir stanečić, 9a5dsz
Sent: Thursday, 1 August 2019 9:03 PM
To: nvda-devel@groups.io
Subject: Re: [nvda-devel] Extra checks for Pull Requests

Hi Reef,
To hijack the thread…
Why it’s not possible to download the latest alpha?
Somewhere we have 404 errors

From: nvda-devel@groups.io <nvda-devel@groups.io> On Behalf Of Reef Turner
Sent: Thursday, August 1, 2019 8:59 PM
To: nvda-devel@groups.io
Subject: [nvda-devel] Extra checks for Pull Requests

Hi everyone,

We have recently introduced new checks for Pull Requests on GitHub.

Code contributors regularly have to deal with ill-defined and inconsistently enforced code style requirements. Code reviewers spend much of their time reporting minor issues, time that would be better spent looking for architectural problems / product issues / logic errors.

To improve this situation, in future PR builds the Flake8 linter will be run checking any new code that will be introduced with a PR. This can also be run with SCons using `scons lint base=origin/master`. Please see the `tests/lint/readme.md` file for more information.

As part of a PR build, the Flake8 linter will be run checking any new code that will be introduced with a PR. When Flake8 reports the does not comply with it’s configuration, a message from Appveyor will be added as a comment to the Pull Request, and the build will fail. Please note, that in this case the artifacts (PR build executable) will still be available if those steps were successful.

We have included an extension with the linter setup to allow tabs instead of spaces, and my find it necessary to disable other warnings, or introduce workarounds. However, in general we would like to stick as close to the default settings as possible.

Thanks for all your contributions!
--
Reef Turner
Software Developer

www.nvaccess.org
Facebook: https://www.facebook.com/NVAccess
Twitter:@NVAccess









--------------------------------------------------------------------------------

Patrick ZAJDA
 

Hello,


Finaly I fixd it, even if I am really not convinced by this kind of codestyle (PR9707).


Have a good day,


Patrick


Le 01/08/2019 à 22:33, Patrick ZAJDA via Groups.Io a écrit :

Hello Reef,

First of all, nice to see this great news.

It looks like there is something strange, or globalCommand.py must be reviewed :)

The "script" decorator make these new tests failed with this message:

"

E121 continuation line under-indented for hanging indent"

I wanted to test with my pull request as it is not a bug I though it could be good to see what happens.

I might have done something bad, even if I read and the other script with the decorator has the same style as the code I made.

What should I do to pass these tests?

Thanks,

Patrick

Le 01/08/2019 à 20:58, Reef Turner a écrit :

Hi everyone,

 

We have recently introduced new checks for Pull Requests on GitHub.

 

Code contributors regularly have to deal with ill-defined and inconsistently enforced code style requirements. Code reviewers spend much of their time reporting minor issues, time that would be better spent looking for architectural problems / product issues / logic errors.

 

To improve this situation, in future PR builds the Flake8 linter will be run checking any new code that will be introduced with a PR. This can also be run with SCons using `scons lint base=origin/master`. Please see the `tests/lint/readme.md` file for more information.

 

As part of a PR build, the Flake8 linter will be run checking any new code that will be introduced with a PR. When Flake8 reports the does not comply with it’s configuration, a message from Appveyor will be added as a comment to the Pull Request, and the build will fail. Please note, that in this case the artifacts (PR build executable) will still be available if those steps were successful.

 

We have included an extension with the linter setup to allow tabs instead of spaces, and my find it necessary to disable other warnings, or introduce workarounds. However, in general we would like to stick as close to the default settings as possible.

 

Thanks for all your contributions!

--

Reef Turner Software Developer 

 

www.nvaccess.org

Facebook: https://www.facebook.com/NVAccess  Twitter: @NVAccess 

 

--
Patrick ZAJDA
Certification NVDA 2019
--
Patrick ZAJDA
Certification NVDA 2019

Patrick ZAJDA
 

Hello,


You can try this link:


https://ci.appveyor.com/api/buildjobs/vg63itgyy5xrff9m/artifacts/nvda_snapshot_alpha-18266%2C1af87759.exe


Downloaded it three times with success and got it following Reef's instructions.


Hop this help,


Patrick

Le 02/08/2019 à 10:22, Brian's Mail list account via Groups.Io a écrit :
Hmm, well I think I'll wait for a fix. grin.
Brian

bglists@...
Sent via blueyonder.
Please address personal E-mail to:-
briang1@..., putting 'Brian Gaff'
in the display name field.
Newsgroup monitored: alt.comp.blind-users
----- Original Message ----- From: "Reef Turner" <reef@...>
To: <nvda-devel@groups.io>
Sent: Thursday, August 01, 2019 9:28 PM
Subject: Re: [nvda-devel] Extra checks for Pull Requests


Thanks for the heads up. This is due to the changes I made to how appveyor uploads files. I’ll push a fix for this in the morning. For now, if you want the lastest alpha, you will have to download it directly from the appveyor artifacts page.

Here is a direct link for now. Until this is fixed you can do the following:

1. Go to our snapshots page
2. Copy the link  URL for the build, eg ‘alpha-18266,1af87759’ not ‘download’
3. Paste in the address bar and add ‘/artifacts’ and press enter
4. Find the link for ‘nvda_snapshot_alpha-18266,1af87759.exe’ and press enter.

From: zvonimir stanečić, 9a5dsz
Sent: Thursday, 1 August 2019 9:03 PM
To: nvda-devel@groups.io
Subject: Re: [nvda-devel] Extra checks for Pull Requests

Hi Reef,
To hijack the thread…
Why it’s not possible to download the latest alpha?
Somewhere we have 404 errors

From: nvda-devel@groups.io <nvda-devel@groups.io> On Behalf Of Reef Turner
Sent: Thursday, August 1, 2019 8:59 PM
To: nvda-devel@groups.io
Subject: [nvda-devel] Extra checks for Pull Requests

Hi everyone,

We have recently introduced new checks for Pull Requests on GitHub.

Code contributors regularly have to deal with ill-defined and inconsistently enforced code style requirements. Code reviewers spend much of their time reporting minor issues, time that would be better spent looking for architectural problems / product issues / logic errors.

To improve this situation, in future PR builds the Flake8 linter will be run checking any new code that will be introduced with a PR. This can also be run with SCons using `scons lint base=origin/master`. Please see the `tests/lint/readme.md` file for more information.

As part of a PR build, the Flake8 linter will be run checking any new code that will be introduced with a PR. When Flake8 reports the does not comply with it’s configuration, a message from Appveyor will be added as a comment to the Pull Request, and the build will fail. Please note, that in this case the artifacts (PR build executable) will still be available if those steps were successful.

We have included an extension with the linter setup to allow tabs instead of spaces, and my find it necessary to disable other warnings, or introduce workarounds. However, in general we would like to stick as close to the default settings as possible.

Thanks for all your contributions!
--
Patrick ZAJDA
Certification NVDA 2019

 

Patrick,


it might help to mention your concerns with the code style in the pr itself.


Regards,

Leonard

On 2-8-2019 12:05, Patrick ZAJDA via Groups.Io wrote:

Hello,


Finaly I fixd it, even if I am really not convinced by this kind of codestyle (PR9707).


Have a good day,


Patrick


Le 01/08/2019 à 22:33, Patrick ZAJDA via Groups.Io a écrit :

Hello Reef,

First of all, nice to see this great news.

It looks like there is something strange, or globalCommand.py must be reviewed :)

The "script" decorator make these new tests failed with this message:

"

E121 continuation line under-indented for hanging indent"

I wanted to test with my pull request as it is not a bug I though it could be good to see what happens.

I might have done something bad, even if I read and the other script with the decorator has the same style as the code I made.

What should I do to pass these tests?

Thanks,

Patrick

Le 01/08/2019 à 20:58, Reef Turner a écrit :

Hi everyone,

 

We have recently introduced new checks for Pull Requests on GitHub.

 

Code contributors regularly have to deal with ill-defined and inconsistently enforced code style requirements. Code reviewers spend much of their time reporting minor issues, time that would be better spent looking for architectural problems / product issues / logic errors.

 

To improve this situation, in future PR builds the Flake8 linter will be run checking any new code that will be introduced with a PR. This can also be run with SCons using `scons lint base=origin/master`. Please see the `tests/lint/readme.md` file for more information.

 

As part of a PR build, the Flake8 linter will be run checking any new code that will be introduced with a PR. When Flake8 reports the does not comply with it’s configuration, a message from Appveyor will be added as a comment to the Pull Request, and the build will fail. Please note, that in this case the artifacts (PR build executable) will still be available if those steps were successful.

 

We have included an extension with the linter setup to allow tabs instead of spaces, and my find it necessary to disable other warnings, or introduce workarounds. However, in general we would like to stick as close to the default settings as possible.

 

Thanks for all your contributions!

--

Reef Turner Software Developer 

 

www.nvaccess.org

Facebook: https://www.facebook.com/NVAccess  Twitter: @NVAccess 

 

--
Patrick ZAJDA
Certification NVDA 2019
--
Patrick ZAJDA
Certification NVDA 2019