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

Add more/better references for LFS infrastructure #2481

Closed
HHHartmann opened this issue Sep 7, 2018 · 8 comments
Closed

Add more/better references for LFS infrastructure #2481

HHHartmann opened this issue Sep 7, 2018 · 8 comments

Comments

@HHHartmann
Copy link
Member

Missing feature

Better and more links to new LFS image build infrastructure (online service and Docker)
At least add/rework

  • Quick start guide does not mention online service and docker.
  • the "Compiling Lua on your PC for Uploading" in upload.md mentions LFS at the very end but even I over-read it the first time reading. The section should be restructured and clearly show LFS as favorized way to do it. Also mention the different ways to get an lfs image.
  • In "Building the firmware" the LFS section should be more of a teaser and in short explain what to do, As it is in the other sections too.

Justification

Start using LFS would be easier

Workarounds

Find the bits by digging deeper.

@jmd13391
Copy link

jmd13391 commented Sep 7, 2018

Hi @HHHartmann, Along these lines, please expand on your #2457 (comment). I am getting ready to make a small contribution to the NodeMCU Quick Start docs and have been unable to get the process you call out in that comment to work. Specifically, there is no "build" script in the tools folder. The closest script I could find is "pr-build.sh" and along with it being inappropriate for a entry level build, it fails with errors changing to \opt\app in addition to having an undefined TRAVIS_BUILD_DIR variable.

Your feedback on this would go a long way to me sorting-out the Docker build process under Windows.

-Joe

@marcelstoer
Copy link
Member

marcelstoer commented Sep 8, 2018

Gregor, I started working on something in my fork two nights ago: https://msr-nodemcu.readthedocs.io/en/feat-doc-quickstart/en/getting-started/
Source: https://github.com/marcelstoer/nodemcu-firmware/blob/feat/doc-quickstart/docs/en/getting-started.md

One of the next steps will be to pull the "Quick start" chapter out of the LFS whitepaper and embed it there.

Feedback and contributions welcome.

@marcelstoer
Copy link
Member

marcelstoer commented Sep 13, 2018

@TerryE @jmd13391 @HHHartmann 80% of what I've had in mind is now available in my new "Getting Started" at https://msr-nodemcu.readthedocs.io/en/feat-doc-quickstart/en/getting-started/. The "Quick start" in the LFS whitepaper was moved over.

It has its shortcomings but I think it's still a major improvement. WDYT?

@HHHartmann
Copy link
Member Author

@marcel that is a major improvement and it is in prominennt place for easy discovery.

I have only some small comments.
It would be easier to do them against a PR.

In the take at the beginning i miss a small Agenda explaining the blue coloring of some cells. I spotted trading the caution box after the first 2 sentences. Btw. what does wrt mean in the 3. Sentence?

The LFS table should be separated from the other one.

A bit further down you write "Build LFS firmware". I feel it should be named "Build LFS enabled firmware".

Ok. Almost arrived at my work. More comments later.

@marcelstoer
Copy link
Member

It would be easier to do them against a PR.

That was my plan if you guys agree that this should land in upstream. So, it's fine if you hold back with the detailed comments until it's up for PR. If I get more encouraging feedback in the next let's say 12h I'll create a PR which ideally might then land just before the master snap.

@jmd13391
Copy link

@marcelstoer Let 'er rip! We can fine tune we go along. Nice work!

@TerryE
Copy link
Collaborator

TerryE commented Sep 14, 2018

@marcelstoer @ 20,000ft this is a very good document and well placed in the hierarchy. I have reviewed it and will add the review comments once you raise the PR.

(Incidentally my preferred way to do this is to view the page without a page style and cut and past the rich text into Writer, then turn on mark-up so I can do the comments and suggested corrections using in-place WYSIWYG redlining, before then transcribing the comments back into the MD file review in gitub. If anyone has a better methods of marking up markup files for review purposes, then I would be grateful for any suggestions.)

I'll create a PR which ideally might then land just before the master snap.

My main concern is that 12 hrs will turn into 24 + review cycles and merge will add another 3-4 days delay before we push to master. I am taking a few weeks break next Friday, so I doubt that the next batch of LFS enhancements which are stalled and an unmerged PR won't make this cut so won't get released into dev for maybe another month. My preference would be to make the dev->master cut now as it is due. Since this is a documentation-only change and really only impacts the readio site view, surely we could easily push it to master after the dev->master cut, and this would allow me to release the functional improvements in parallel.

@marcelstoer
Copy link
Member

PR ready at #2487

@TerryE TerryE closed this as completed Sep 17, 2018
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

No branches or pull requests

4 participants