Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify ESP8266 build instructions #1879

Closed

Conversation

SpotlightKid
Copy link
Contributor

  • Add instructions to get external dependencies to esp8266/README.md.
  • Add reference to ESP8266 README in main README.
  • Make section on external deps in main README more generic.
  • Move unix section behind it.

@SpotlightKid SpotlightKid force-pushed the esp8266-readme-install branch 3 times, most recently from ccaa603 to d4f7e45 Compare March 8, 2016 18:10
* Add instructions to get external dependencies to esp8266 README.
* Add reference to esp8266 README in main README.
* Make section on external deps in main READEM more generic.
* Move section on unix port behind external deps section.
@SpotlightKid
Copy link
Contributor Author

Any opinions on this? I'm asking because the missing instruction to init the git submodules in the esp8266 readme has already tripped up several people on the forum.

@dpgeorge
Copy link
Member

I'm happy to merge the change to esp8266/README.md; that should be enough (you anyway must read that file to know how to build it).

@pfalcon
Copy link
Contributor

pfalcon commented Mar 10, 2016

I'm happy to merge the change to esp8266/README.md; that should be enough (you anyway must read that file to know how to build it).

That's exactly what I wanted to do (but didn't get to). The rest of changes as they are complicate unix build. They would be grounded if unix is switched to require external modules (axTLS), and it's a shame it wasn't still, but the reason for that is exactly not to complicate unix build for newcomers. And for now we have enough changes with esp8266 port to put other "breaking" changes on hold.

@SpotlightKid
Copy link
Contributor Author

I'm not sure what you mean with your remark about the unix build. All I did was put the section "External dependencies" before the section "The Unix version" so that all sections on different ports follow each other, and changed the first and fourth paragraph of the "External dependencies" section to make it clear that the part about git submodules not only applies to the unix build but also to other ports, namely (atm) the esp8266 port.

@SpotlightKid
Copy link
Contributor Author

Maybe the main README should just have a short reference to the README in the port's subdirectory for each port anyway.

@dpgeorge
Copy link
Member

Maybe the main README should just have a short reference to the README in the port's subdirectory for each port anyway.

No, that will be too many paragraphs just saying "see here".

@dpgeorge
Copy link
Member

Changes to esp8266/README.md were merged in 3d0e3a3.

The main readme needs to stay lean and efficient, like uPy itself :)

@dpgeorge dpgeorge closed this Mar 11, 2016
@SpotlightKid
Copy link
Contributor Author

Ok, thanks! I might do another PR to move that unix port specific stuff into its own README then ;)

@pfalcon
Copy link
Contributor

pfalcon commented Mar 11, 2016

@SpotlightKid : Alternatively, feel free to leave it to project maintainers ;). There're a lot of places where community can help, and few places which better be left as responsibility of the maintainers. You of course welcome to argue your cause (but then please start with arguments, not patches).

@SpotlightKid
Copy link
Contributor Author

To me, a PR is an invitation for a discussion. :) I'm not bothered if a PR is rejected.

@pfalcon
Copy link
Contributor

pfalcon commented Mar 11, 2016

Sounds good!

@SpotlightKid SpotlightKid deleted the esp8266-readme-install branch November 26, 2017 16:53
tannewt added a commit to tannewt/circuitpython that referenced this pull request May 14, 2019
…ge-fixes

Handle truth values; speed up smallint checks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants