Skip to content

Markus' PEN review.#86

Merged
loumir merged 8 commits intoivoa-std:mainfrom
msdemlei:markus-review
Apr 29, 2026
Merged

Markus' PEN review.#86
loumir merged 8 commits intoivoa-std:mainfrom
msdemlei:markus-review

Conversation

@msdemlei
Copy link
Copy Markdown
Contributor

This is in particular fixing some registry aspects (e.g., we don't want mixed-case identifiers, and it's important to have a sample record).

You're sure calling this thing obscore_radio is so preferable over the obs_radio that's already present in some services? It's also more chars to type...

I am 100% sure that registering an obs_radio table musst entail the presence of ivoa.obscore. Anything else would be frivolous and useless. Let's not specify for that, and let's use a sane discovery query.

I'm also fixing a few language things in the first part but then ran out of time while reviewing.

I have also added a couple of todos where I couldn't really work out what is being said.

msdemlei and others added 6 commits April 16, 2026 19:47
This is in particular fixing some registry aspects (e.g., we don't want
mixed-case identifiers, and it's important to have a sample record).

You're sure calling this thing obscore_radio is so preferable over
the obs_radio that's already present in some services?  It's also more
chars to type...

I am 100% sure that registering an obs_radio table musst entail the presence
of ivoa.obscore.  Anything else would be frivolous and useless.  Let's not
specify for that, and let's use a sane discovery query.

I'm also fixing a few language things in the first part but then ran out of
time while reviewing.
Updated AUTHOR_EMAIL to include additional contact.
Removed a todo comment regarding the citation of an IVOA note and added a note about the document being uploaded.
Removed a comment about explaining the importance of certain parameters for predicting minimal baseline.
@loumir loumir self-requested a review April 17, 2026 11:30
loumir
loumir previously approved these changes Apr 17, 2026
Copy link
Copy Markdown
Collaborator

@loumir loumir left a comment

Choose a reason for hiding this comment

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

thanks for the various updates .
I have added a few typos corrections and complement as responses to the \todo.

Clarified the access method for visibility data maps and corrected the language regarding their representation in ObsDataset.
loumir
loumir previously approved these changes Apr 23, 2026
Copy link
Copy Markdown
Collaborator

@loumir loumir left a comment

Choose a reason for hiding this comment

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

changed text for uv maps access

Corrected a comment regarding the exposure of maps via DataLink service.
@Bonnarel Bonnarel assigned Bonnarel and unassigned Bonnarel Apr 27, 2026
@Bonnarel Bonnarel self-requested a review April 27, 2026 22:07
Copy link
Copy Markdown
Collaborator

@Bonnarel Bonnarel left a comment

Choose a reason for hiding this comment

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

I approve this Pull request according it takes into account Mireille's comments

@Bonnarel
Copy link
Copy Markdown
Collaborator

This is in particular fixing some registry aspects (e.g., we don't want mixed-case identifiers, and it's important to have a sample record).
OK thanks

You're sure calling this thing obscore_radio is so preferable over the obs_radio that's already present in some services? It's also more chars to type...

This is extension to obscore, should be clear in the name

I am 100% sure that registering an obs_radio table musst entail the presence of ivoa.obscore. Anything else would be frivolous and useless. Let's not specify for that, and let's use a sane discovery query.

OK

I'm also fixing a few language things in the first part but then ran out of time while reviewing.

Thanks

I have also added a couple of todos where I couldn't really work out what is being said.

@loumir loumir merged commit 825ccb7 into ivoa-std:main Apr 29, 2026
1 check passed
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