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

Adding where to new att.locatable to address #1769 #1958

Closed
wants to merge 16 commits into from

Conversation

ebeshero
Copy link
Member

I've moved @where to a new att.locatable and I'm hoping we can look at / improve my new explanation of it. I'm also hoping we can add some translations to other languages here.

@ebeshero ebeshero added this to the Guidelines 3.7.0 milestone Jan 16, 2020
@ebeshero ebeshero changed the title Adding where to new att.locatable for #1769 Adding where to new att.locatable to address #1769 Jan 16, 2020
@ebeshero
Copy link
Member Author

Okay, we're really passing build tests now. Let's take a look and see if this is what we want to add to the Guidelines.

Copy link
Member

@peterstadler peterstadler left a comment

Choose a reason for hiding this comment

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

the bibliography shows a lot of whitespace changes. Is it somehow possible to reset these and only add the needed changes?

P5/Source/Guidelines/en/ND-NamesDates.xml Show resolved Hide resolved
P5/Source/Specs/att.locatable.xml Outdated Show resolved Hide resolved
P5/Source/Specs/att.locatable.xml Outdated Show resolved Hide resolved
P5/Source/Specs/att.locatable.xml Outdated Show resolved Hide resolved
@@ -173,7 +104,7 @@ $Id$
</castList>
</performance>
<!-- ... -->
<stage type="entrance"><move who="#bellaf" type="enter" where="L" perf="#perf1"/> Enter
<stage type="entrance"><move who="#bellaf" type="enter" where="#L" perf="#perf1"/> Enter
Copy link
Member

Choose a reason for hiding this comment

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

I think we'll need an example for resolving the where="#L" as all other targets of references ("#bellaf" and "#perf1") are provided.

P5/Source/Specs/move.xml Outdated Show resolved Hide resolved
@ebeshero
Copy link
Member Author

ebeshero commented Jan 17, 2020

@peterstadler @martindholmes @martinascholger @TEITechnicalCouncil Thanks for your help with this PR! I've made some changes as we discussed to expand the examples, so we now have some lists of stage movements for @where to point to. I've also made some revisions to the wording as suggested, and I restored the spacing I think as it was before in the BIB, and just added my new entry. How does this look?

@ebeshero
Copy link
Member Author

Edited the above to separate this part out: If we decide this is okay to merge into dev, can I get some help with figuring out how to do something like a deprecation of the old way of doing things, like we discussed in our meeting? @sydb : I'm trying to remember how we do this.

@ebeshero ebeshero requested a review from sydb January 17, 2020 13:20
@ebeshero
Copy link
Member Author

Just an update on this branch--I think we resolved the requested changes now, but there's an outstanding issue (not on the reviews I think?) about how we talk to people about their use of the new datatype on @where which might invalidate existing values. I could use a little help with that, but I'll try something on it later today.

@ebeshero
Copy link
Member Author

VF2F decision: We are thinking of better ways to handle the expression of stage movement, and think att.locatable is probably not the best approach to take now. Closing and not merging.

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