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

Removing dev096 dev120 dev140 branches #716

Closed
TerryE opened this issue Oct 31, 2015 · 16 comments

Comments

Projects
None yet
5 participants
@TerryE
Copy link
Collaborator

commented Oct 31, 2015

All are no longer being maintained, and their existence simply confuses developers who want to use nodeMCU.

  • dev096 with the exception of a couple of tweaks by @dnc40085 ( bfa1749 and bea5e90 ) dev includes all relevant changes and even these have had parallel work.
  • dev120 only changes are @vowstar's work that has been superseded in dev by @jmattsson Johny's work.
  • dev140 is a clone of the DiUS/dev140 branch and again all of this work has been merged into dev.

MY recommendation is that I delete these tomorrow on Monday, unless any other committer says no. I'll keep a copy of these on my local copy just in case we want to refer to them.

Comments? Objections?

The next job will be to reconcile dev against master.

@marcelstoer

This comment has been minimized.

Copy link
Member

commented Oct 31, 2015

What a relief, thanks.

@dnc40085

This comment has been minimized.

Copy link
Contributor

commented Oct 31, 2015

I think that's a great idea.

As for the event monitor tweaks... I have been working on exposing the new event monitor included in the SDK, but I'm not sure how to make a clean swap from the one I wrote previously and the new one without causing too many waves.

I could include a preprocessor directive to allow switching between new event monitor and old one so people who still want to use the old one have the option.

On a side note, as a user of the 512k ESPs, I was wondering...since with every new SDK there is less and less flash available to the NodeMCU firmware and to the end user, Would it not be a good idea to have a way to choose which functions, within modules, to include in a build similar to the file "user_modules.h"? Should this be in it's own issue?

@TerryE

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 31, 2015

Would it not be a good idea to have a way to choose which functions, within modules, to include in a build similar to the file "user_modules.h"?

It just gets too complex. There are some exceptions. E.g. crypto is largely a wrapper around ROM / SDK functions and has a very light footprint -- apart from the fact that it includes SHA2 implementation, but this can be removed by a define if you are happy to live with MD5 and SHA1.

I am not sure how many people use the event monitor yet (I do), but for now I suggest that you would offer both APIs with a user_config option to select which the programmer wants and perhaps deprecate the old version with a warning that we are going to remove it in X months to avoid the longer term maintenance issues.

@dnc40085

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2015

Thanks for the suggestions.
Which should be default? the new or old event monitor?
How about all the disconnect reasons? Should I make Lua table entries, 28 in total, for all disconnect reasons adding a minimum of 420-620 bytes of text?
If so, should I also add a method to exclude reason entries if desired?

@TerryE

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 1, 2015

@dnc40085, I'll have to look through your fork because I don't know what your new proposed API is. As far as properties go, why not use a metatable and an __index function? Where are you publishing your new event monitor code. I can't find it in your nodemcu fork.

@dnc40085

This comment has been minimized.

Copy link
Contributor

commented Nov 1, 2015

@TerryE My apologies, I was working on it locally.
I've created a new branch named dev_New_event_monitor on my github fork.

As far as properties go, why not use a metatable and an __index function?

I'm still learning Lua/Lua C API, could you elaborate on your suggestion?

@TerryE

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 2, 2015

If you use pairs to enumerate _G then you'll not find any rotables listed, Yet they are still accessible. Why? review app/lua/lbaselib.cand also net uses these techniques; see app/lua/lbaselib.c.

@TerryE

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 2, 2015

All, you will see that dev120 and dev140 are no more. Dev includes all 1.4 code. I'll wait until later just in case anyone can think of a reason for not removing 096.

@saitejal

This comment has been minimized.

Copy link

commented Nov 2, 2015

@TerryE dev096 seems to be the current stable branch. Maybe it is a good idea to leave 096 as it is until we get a prebuild for 140.

@TerryE

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 2, 2015

@saitejalakkimsetty. No, don't confuse tags and branches in git. dev096 isn't the current stable branch. Tag 0.9.6-dev_20150627 was the last "stable" build. You can still fetch this source whether or not the branch exists. No one is working on dev096 and we won't be accepting any more PRs for it. However, I will delay deleting this branch another couple of days in case I get other positions logged.

@saitejal

This comment has been minimized.

Copy link

commented Nov 2, 2015

@TerryE Sounds good.

@karrots

This comment has been minimized.

Copy link
Contributor

commented Nov 2, 2015

My question is similar to one expressed above. How do we get master and dev in sync? There are recent patches to master that need to end up in dev for example.

@TerryE

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 2, 2015

The next job will be to reconcile dev against master.

Say no more 😄

@karrots

This comment has been minimized.

Copy link
Contributor

commented Nov 3, 2015

Cool

@TerryE

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 7, 2015

Done. dev096 is no more and we are now down to dev and master

@TerryE

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 8, 2015

All done and no comments so this one is closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.