Skip to content

V3 actual consistent naming - #1

Merged
dennwc merged 6 commits into
dennwc:v3from
bzz:v3-actual-consistent-naming
Oct 17, 2018
Merged

dennwc merged 6 commits into
dennwc:v3from
bzz:v3-actual-consistent-naming

Conversation

@bzz

@bzz bzz commented Oct 17, 2018 •

Copy link
Copy Markdown

Refactoring for clarity and consistency, as discussed IRL:

  • NodeExtType => PyNodeExtType, NodeExt => PyNodeExt
  • PyUastType => PyContextType, PyUast => PyContext

Part of the bblfsh#128

Comment thread bblfsh/pyuast.cc Outdated
}

static PyObject *PyUastDecode(PyObject *self, PyObject *args) {
static PyObject *PyContextDecode(PyObject *self, PyObject *args) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Py_Decode, since it's a global method, or PyContextExt_decode since it's a constructor specifically for PyContextExt

@bzz bzz Oct 17, 2018 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

👍 this is a perfect example how we could benefit from new github feature :) https://blog.github.com/category/all/#suggested-changes-public-beta

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

addressed in 2785a60

Comment thread bblfsh/pyuast.cc Resolved

@dennwc dennwc left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good, thanks! :)

@dennwc
dennwc merged commit 9e88733 into dennwc:v3 Oct 17, 2018
@bzz

bzz commented Oct 17, 2018

Copy link
Copy Markdown
Author

🎉 :shipit:

@bzz
bzz deleted the v3-actual-consistent-naming branch October 17, 2018 10:02
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.

2 participants