PierrickKoch
commented
7 years ago - I agree with the
*import being a bad practice in production code, however in the case of "builder script" it makes it easier to setup a simulation. envis not mandatory- agree with
morse run mysim/ - agree with British English
- agree with robot.rotate
- agree for a note on explicit naming
- we definitely need to stick to one of them
- I don't see the problem with L547
once again, thanks for all the review!
severin-lemaignan
Using * imports is bad style. You should show how to use morse without using them.
If the Environment object must be called env, you ought to say so.
In bash, if you type "morse run my" and then tab-complete you'll get "morse run mysim/" which will produce an error. Surely, it could be smart enough to look for "mysim/" and if it doesn't find it fall back to trying "mysim"?
Most of the English seems British, but then you use the American meter rather than British (French!) metre.
On line 231 I don't understand where the atrv object comes from; why isn't it robot.rotate(...)?
Clearly the auto-naming is useful for beginners, but surely you should advise people to explicitly name?
Sometimes morse is written MORSE and other times Morse -- you ought to choose one and keep to it throughout:-)
From line 547 the syntax highlighting for code blocks is wrong: may just be a bug in the github editor though.