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.

Sign up for GitHub

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

Jump to bottom

net: rename net.if.info to net.ifinfo #3033

Merged
merged 1 commit into from
Feb 23, 2020
Merged

net: rename net.if.info to net.ifinfo #3033

merged 1 commit into from
Feb 23, 2020

Conversation

nwf
Copy link
Member

@nwf nwf commented Feb 3, 2020

Reserved words are always reserved in Lua, so let's not have people
typing net["if"]...

  • 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/*.

@nwf nwf requested a review from marcelstoer February 3, 2020 08:57
@nwf
Copy link
Member Author

nwf commented Feb 3, 2020

#2862 was landed a little prematurely, perhaps. Please note that it, and this, are not tested.

@marcelstoer marcelstoer added this to the Next release milestone Feb 3, 2020
@marcelstoer
Copy link
Member

I hope Apple doesn't claim copyright on the term "iface" 😜

@HHHartmann
Copy link
Member

I'd rather name it interface or ifsomething
Or how about changing net.if.info() to net.ifinfo()

@nwf
Copy link
Member Author

nwf commented Feb 9, 2020

ifinfo is OK by me. I'll make that change, write a test, and merge, if that seems OK, @marcelstoer ?

@marcelstoer
Copy link
Member

👍

@nwf
Copy link
Member Author

nwf commented Feb 23, 2020

This is ready for merging; someone care to review just in case? (@marcelstoer ?)

HHHartmann
HHHartmann requested changes Feb 23, 2020
View reviewed changes
Copy link
Member

@HHHartmann HHHartmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove chose from other PR.
From reading the code the rest looks good but haven't tried.

app/modules/net.c Show resolved Hide resolved
marcelstoer
marcelstoer reviewed Feb 23, 2020
View reviewed changes
app/modules/net.c Outdated Show resolved Hide resolved
@nwf
net: rename net.if.info to net.ifinfo
3d3645e 
Reserved words are always reserved in Lua, so let's not have people
typing net["if"]...
@nwf
Copy link
Member Author

nwf commented Feb 23, 2020 •
edited

@HHHartmann What do you mean by "Please remove chose from other PR." ? Ah, github hid the resolved comments from me. Nevermind!

@nwf nwf changed the title net: rename net.if to net.iface net: rename net.if.info to net.ifinfo Feb 23, 2020
@nwf nwf merged commit 31d4714 into nodemcu:dev Feb 23, 2020
@nwf nwf deleted the upstream-net-iface branch February 27, 2020 16:41
marcelstoer pushed a commit that referenced this pull request Jun 9, 2020
@nwf @marcelstoer
net: rename net.if.info to net.ifinfo (#3033)
446c058 
Reserved words are always reserved in Lua, so let's not have people
typing net["if"]...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marcelstoer marcelstoer marcelstoer approved these changes

@HHHartmann HHHartmann HHHartmann approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
June 2020
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@nwf @marcelstoer @HHHartmann