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

naming advice around inj and ext #60

Merged
merged 2 commits into from
Jun 18, 2020
Merged

naming advice around inj and ext #60

merged 2 commits into from
Jun 18, 2020

Conversation

kim-em
Copy link
Collaborator

@kim-em kim-em commented Jun 14, 2020

No description provided.

Copy link
Member

@jcommelin jcommelin left a comment

Choose a reason for hiding this comment

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

lgtm

@jcommelin
Copy link
Member

I think it's good to codify what has been an implicit naming practice already. I'm fine with merging this. @bryangingechen @robertylewis @digama0 any comments?

@bryangingechen
Copy link
Collaborator

I'm happy with merging this too.

@robertylewis
Copy link
Member

I'm not entirely up on the naming conventions but this looks fine to me. I don't think the trailing ###s are necessary.

@bryangingechen
Copy link
Collaborator

I don't think the trailing ###s are necessary.

They're just there for consistency with the rest of the file, I think. We can get rid of them in a cleanup pass later on. See #62 as well as the duplicated variable names table in style.md.

@bryangingechen bryangingechen merged commit 04a5a82 into newsite Jun 18, 2020
@bryangingechen bryangingechen deleted the naming-inj-ext branch June 18, 2020 22:17
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.

5 participants