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

[Small PR] Made Labels more meaningful in GlassBR #914

Merged
merged 4 commits into from
Jul 20, 2018
Merged

Conversation

samm82
Copy link
Collaborator

@samm82 samm82 commented Jul 19, 2018

As per smiths/caseStudies#106, labels for T1 and T2 were changed to contain useful information.

@szymczdm
Copy link
Collaborator

@samm82 I believe some of the other PRs are causing conflicts here, can you please sort them out?

@szymczdm szymczdm merged commit 6799939 into master Jul 20, 2018
@szymczdm szymczdm deleted the t1t2Labels branch July 20, 2018 14:46
@smiths
Copy link
Collaborator

smiths commented Jul 21, 2018

I mentioned in comments that the Drasil names that have t1 and t2 in them are still not great names. The order of t1 and t2 is arbitrary.

@samm82
Copy link
Collaborator Author

samm82 commented Jul 23, 2018

Addressed in the addGlassDD6Notes branch (it was a small fix so I just tacked it on to the branch)

@smiths
Copy link
Collaborator

smiths commented Jul 23, 2018

Looks good. I still like a prefix (like T) that tells me that this is a theoretical model, but it is best that the examples all be consistent.

@samm82
Copy link
Collaborator Author

samm82 commented Jul 23, 2018

a) So do you want me to add the prefix?
b) Should I change the other examples with a similar fix?

@smiths
Copy link
Collaborator

smiths commented Jul 23, 2018

Hold off for now. It is more of a stylistic preference, and we haven't really converged on a style for Drasil code. Eventually we may want to put something in the contributor's guide related to this, but our conventions aren't yet clear. I think we can close this issue for now.

JacquesCarette pushed a commit that referenced this pull request Jul 23, 2018
* Added glass type notes to DD6 - closes #751

* Renamed t1 and t2 to pb and lr in function names as per comments on #914
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.

4 participants