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

Text changes for issue #388 #535

Merged
merged 4 commits into from Sep 10, 2018
Merged

Text changes for issue #388 #535

merged 4 commits into from Sep 10, 2018

Conversation

xatapult
Copy link
Contributor

@xatapult xatapult commented Sep 7, 2018

I tried to describe map attributes and moved the section about option attributes to the right location

gimsieke
gimsieke reviewed Sep 7, 2018
View reviewed changes
Copy link
Contributor

@gimsieke gimsieke left a comment

Choose a reason for hiding this comment

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

I wonder whether we want to call them map attributes only informally. Shouldn’t we make a proper termdef with firstterm of it?
If the text stays as it is, at least use typographic quotes (“map attribute” instead of "map attribute").

@xatapult
Copy link
Contributor Author

I now made "map-attribute" an official termdef. Better?

gimsieke
gimsieke approved these changes Sep 10, 2018
View reviewed changes
Copy link
Contributor

@gimsieke gimsieke left a comment

Choose a reason for hiding this comment

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

Approve, but consider changing “datatype” to “sequence type”

@xatapult
Datatype is now sequence type
c5f6c57 
Changed the wordings to make it adhere to the definition of AVT
@xatapult
Copy link
Contributor Author

Ok, datatype is now sequence type.
Also changed the wording a bit because I saw the definition of AVT was different then I thought it was.

gimsieke
gimsieke approved these changes Sep 10, 2018
View reviewed changes
Copy link
Contributor

@gimsieke gimsieke left a comment

Choose a reason for hiding this comment

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

♫♪
Ah you can merge if you want to, you can leave your peers behind
Cause your peers don't merge and if they don't merge
Well they're are no peers of mine
…
— Men At Work

@gimsieke
Copy link
Contributor

Men Without Hats, apparently…

@xatapult
Copy link
Contributor Author

Aren't we all (in the committee). I haven't seen a hat on our heads in Leipzig :-)

@xatapult xatapult merged commit 376777d into xproc:master Sep 10, 2018
@xatapult xatapult deleted the option-attributes branch September 12, 2018 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gimsieke gimsieke gimsieke approved these changes

@ndw ndw Awaiting requested review from ndw

@xml-project xml-project Awaiting requested review from xml-project

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@xatapult @gimsieke