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

3 Fixes for 3 issues discovered by M.Taylor #4

Merged
merged 3 commits into from
May 5, 2020

Conversation

Bonnarel
Copy link
Contributor

Not familiar with this. Hope this change proposal is correct. Could be the base of an erratum

@mbtaylor
Copy link
Member

I'm happy with the fix for issue #1 .

The fix for #2 needs changing - the word "fixed" is OK but the "{\em [?...?]}" is not supposed to be there (I just added that markup to emphasise that there was a problem).

Issue #3 - really? The edit in the PR removes the ref attribute from the ID PARAM, so I can't see where its value would ever come from, or what purpose it serves. I thought the change would be to add ID="primaryID" to one of the columns in the table. But maybe I don't understand self-describing services. Can somebody else review?

@Bonnarel
Copy link
Contributor Author

Bonnarel commented Oct 17, 2019 via email

Copy link
Collaborator

@pdowler pdowler left a comment

Choose a reason for hiding this comment

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

One small change.

DataLink.tex Outdated
@@ -944,7 +944,7 @@ \subsection{Example: Service Descriptor for VOSpace-2.0}
</RESOURCE>
\end{verbatim}
Since the capability being described is RESTful and inputs are
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the vospace service descriptor example, I would just remove "and inputs are fixed" because it doesn't add anything.

DataLink.tex Outdated Show resolved Hide resolved
@Bonnarel
Copy link
Contributor Author

I modified the change for REStful according to Pat's demand. This text is for an erratum, I guess

@Bonnarel
Copy link
Contributor Author

This change is for RESTful fix. All is valid for an erratumon 1.0

pdowler
pdowler previously approved these changes Feb 10, 2020
@Bonnarel
Copy link
Contributor Author

Bonnarel commented May 5, 2020

There were 3 changes in this PR. 2 were typos or unachieved sentences. they are still to be changed in the master. The third was an error in the "Self-described" service. But this subsection has been totally changed (the example is another service). So the change is now irrelevant to the PR. this has been solved. But it still remain the need to create errata for the 3 "bugs" in 1.0.

@pdowler pdowler merged commit 1a33fc3 into ivoa-std:master May 5, 2020
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.

3 participants