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 LFS size build info means capacity #3022

Merged
merged 1 commit into from
Feb 1, 2020

Conversation

mk-pmb
Copy link
Contributor

@mk-pmb mk-pmb commented Jan 29, 2020

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

Trivial clarification that the LFS build info means its capacity. For a while I was unsure about it because mine shows zero, so I was wondering if it might mean the amount used, or the start position.

@mk-pmb mk-pmb changed the base branch from master to dev January 29, 2020 14:30
@mk-pmb mk-pmb changed the title Clarify lfs size Clarify LFS size build info means capacity Jan 29, 2020
@HHHartmann
Copy link
Member

The branch also has to be based on dev. Maybe you can rebase it.

@mk-pmb
Copy link
Contributor Author

mk-pmb commented Jan 29, 2020

Base branch should be fixed now. At first I had assumed the checkbox was about the Git tree, not the Github UI.

@marcelstoer
Copy link
Member

I had assumed the checkbox was about the Git tree, not the Github UI.

They go hand-in-hand.

@marcelstoer marcelstoer added this to the Next release milestone Jan 31, 2020
@HHHartmann
Copy link
Member

This seems to be ok unless someone is parsing the output. In that case it would break that parser.

@marcelstoer marcelstoer merged commit 49f25bd into nodemcu:dev Feb 1, 2020
@TerryE
Copy link
Collaborator

TerryE commented Apr 25, 2020

I missed this one sorry. I've was just merging these changes and in doing do reviewed this node.c code. @HHHartmann there are weaknesses in this approach of mining data from the make to generate runtime configuration data:

  1. This is denormalising information that is available elsewhere, for example you can enumerate the tables in ROM to list all included modules and other file and node API calls which give the correct runtime configuration values.
  2. The answers returned can be just wrong. For example the LFS region size is based on the value in the Partition Table which can be different to the build default as this can be changed by editing the Partition Table after imaging or by the node.setpartitiontable() API call.

marcelstoer pushed a commit that referenced this pull request Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants