Skip to content

Make mkdir.py work under python3 versions below 3.5#8194

Merged
d-a-v merged 13 commits intoesp8266:masterfrom
gneiss15:master
Jul 9, 2021
Merged

Make mkdir.py work under python3 versions below 3.5#8194
d-a-v merged 13 commits intoesp8266:masterfrom
gneiss15:master

Conversation

@gneiss15
Copy link
Contributor

@gneiss15 gneiss15 commented Jul 4, 2021 •
edited
Loading

Because early versions of python3 did not have the optional arg "exist_ok" for "pathlib.Path(...).mkdir(...)" a build under this versions will abort with an error message. This PR will modify the python script so that it works even under python 3.4 (and below).

gneiss15 and others added 7 commits July 1, 2021 16:05
@gneiss15
Create pins_arduino.h
f5530b9
@gneiss15
Update boards.txt.py
0548a44
@gneiss15
Update boards.txt
f5ecd9a
@gneiss15
done a "boards.txt.py --allgen"
46b0a2c
@gneiss15
Removed the definition of LED_BUILTIN from variants/wifi_kit_8/pins_a…
82e94fa 
…rduino.h, because this board has no build in lED
@gneiss15
Make makdir.py work unter python3 versions below 3.5
73eb2c8 
Because early versions of python3 did not have the optional arg "exist_ok" for "pathlib.Path(...).mkdir(...)" a build under this versions will abort with an error message.
This PR will modify the python script so that it works even under python 3.4 (and below).
@gneiss15
Merge branch 'esp8266:master' into master
8d17aee
@gneiss15 gneiss15 changed the title Make makdir.py work unter python3 versions below 3.5 Make makedir.py work unter python3 versions below 3.5 Jul 4, 2021
@gneiss15 gneiss15 changed the title Make makedir.py work unter python3 versions below 3.5 Make mkdir.py work unter python3 versions below 3.5 Jul 5, 2021
@gneiss15 gneiss15 changed the title Make mkdir.py work unter python3 versions below 3.5 Make mkdir.py work under python3 versions below 3.5 Jul 6, 2021
earlephilhower
earlephilhower requested changes Jul 8, 2021
View reviewed changes
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't change indents with patches. This and the other PY scripts are 4-space indented. Spacing changes like this make it hard to review/sync up.

Also, this seems like a spot where cascaded try (3.5+ version) except all (<3.5 version) would be better than examining python version numbers. Or, if the <3.5 version of Path.mkdir works on >3.5, then just change it to use that API no matter what.

@gneiss15
Copy link
Contributor Author

gneiss15 commented Jul 9, 2021 •
edited
Loading

  1. Idents:
    I corrected this. It's something that has changed by using my favorite editor.

  2. Cascaded try
    I don't thing that a nested try will be better here (because the function that fails is inside a loop, which will imply an exception on every iteration).
    A try will have its advantage if it can't be precisely determined when the function will fail.
    Here it is a well know language feature that was added with version 3.5, so the (original) call to:
    pathlib.Path( ..).mkdir(.., exist_ok = True )
    will fail on all versions below 3.5

  3. Or, if the <3.5 version of...
    OK, that was (IMHO) a better approach.
    I had leave the original inside, because I would have changed as less as possible.

OK, I will upload a version that only use the <3.5 version.

earlephilhower
earlephilhower approved these changes Jul 9, 2021
View reviewed changes
Copy link
Collaborator

@earlephilhower earlephilhower left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is much cleaner!

@d-a-v d-a-v merged commit 95c6fbb into esp8266:master Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@earlephilhower earlephilhower earlephilhower approved these changes

@d-a-v d-a-v d-a-v approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gneiss15 @earlephilhower @d-a-v