search

KiCad / kicad-library-utils

Some scripts for helping with library development
GNU General Public License v3.0
128 stars 95 forks source link

Error following modification of check scripts #292

Closed myfreescalewebpage closed 5 years ago

myfreescalewebpage commented 5 years ago

Following https://github.com/KiCad/kicad-library-utils/pull/274, I have the result shown below on https://github.com/KiCad/kicad-symbols/pull/1601:

Capture

@poeschlr @fauxpark please have a look, thanks. Joel

fauxpark commented 5 years ago

Odd, Travis is still using Python 2...

fauxpark commented 5 years ago

Since build #5023 Travis has been using Python 3 with no complaints.

fauxpark commented 5 years ago

@myfreescalewebpage this build/PR is quite old, but only ran recently, evidently in the period between #274 being merged and the update to Python 3. Did you re-run this build to test it out?

myfreescalewebpage commented 5 years ago

@fauxpark it was the last time run 2 days ago. The PR is now merged and we are now more able to restart the run (it fails because PR is merged).

myfreescalewebpage commented 5 years ago

Seems not to be present on the last run in the pull request I'm currently reviewing. Closing the issue at the moment.

fauxpark commented 5 years ago

I think this is actually expected behaviour if Travis pulls the .travis.yml from the same point in the Git history as the PR. @poeschlr advised not to bother with support for Python 2 in this rule, or the check scripts in general.

So, it doesn't fail because my PR was merged, it fails because it's trying to use Python 2 to check this rule.

myfreescalewebpage commented 5 years ago

@fauxpark seen again today 2019-30-03, reopening the issue. How have you made your previous modifications ? Python 3 ? You should be compatible with Python 2.....

myfreescalewebpage commented 5 years ago

@poeschlr I'm not sure but it seems that:

fauxpark commented 5 years ago

That is interesting. Evidently Travis pulls the .travis.yml from current master for a new build (regardless of where the PR was branched off of), but when rerunning an existing build the Python version is already stored somewhere in the metadata for the build so it simply uses that one.

@myfreescalewebpage this thread appears to offer a solution: https://travis-ci.community/t/can-restart-build-to-use-latest-travis-yaml-configuration/1384

myfreescalewebpage commented 5 years ago

@fauxpark the main issue is that it is not "my" pull request, this is the pull requests of people contributing to the library that we are reviewing... Keeping compatibility with python2 should be very great, isn't it possible ?

fauxpark commented 5 years ago

It is possible, in fact that's what my PR did originally, however Rene seemed to be saying not to bother: https://github.com/KiCad/kicad-library-utils/pull/274#discussion_r253264628

So I defer to him as to what to do next.

poeschlr commented 5 years ago

Yes travis is stuck at python 2 until you close and reopen the pull request. I know this is annoying especially as we now kind of rely on python 3 for the tests. (I did not know about this travis restriction when i merged the changes. Otherwise i would have updated to python 3 a bit before merging the changes that require python 3.)

Edit to make it clear: only pull requests that existed before my changes are stuck at python 2. I would even assume that only commits that where made before that are stuck. Meaning users would never see problems as they can not restart travis in any way other than either adding a new commit or opening and closing the pull request both of which get travis over to python 3. This of course means that we maintainers for now also need to restart travis by closing and reopening the pull request instead of going via the travis interface.

And no python 2 is dead. I am not prepared to invest any time in getting any script compatible with both. (We are at least 3 years late with this decision. I do not quite understand why we even bothered to have them python 2 compatible in the past. This just added additional effort for no real tangible gain)

myfreescalewebpage commented 5 years ago

@poeschlr is it tested that closing/opening the PR will restart travis with the new configuration, i.e. running python3 ?

myfreescalewebpage commented 5 years ago

Just confirmed, the issue can be solved by closing/opening the pull request. Closing this issue.