Skip to content

TAPHandle is CDS/VOFrance - #118

Merged
molinaro-m merged 4 commits into
ivoa:mainfrom
msdemlei:taphandle-is-cds
Sep 2, 2026
Merged

molinaro-m merged 4 commits into
ivoa:mainfrom
msdemlei:taphandle-is-cds

Conversation

@msdemlei

@msdemlei msdemlei commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@JeremyMcCormick JeremyMcCormick left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested a few minor changes.

Comment thread content/astronomers/applications.md Resolved
@JeremyMcCormick

JeremyMcCormick commented Jun 2, 2025 •

Copy link
Copy Markdown
Collaborator

@msdemlei I have invited you to the Web Editors team so that you can make PRs on branches going forward. This is preferred because our CI which deploys the test site doesn't work on forks due to authentication issues.

The fork is fine for this PR since it is such a minor change, but please use branches on the main repo going forward to suggest updates.

@lmichel

lmichel commented Jun 2, 2025

Copy link
Copy Markdown

TAPHandle has been developed by the SSC XMM-Newton team in Strasbourg.
The correct label is: Strasbourg Observatory - SSC XMM-Newton

@JeremyMcCormick

JeremyMcCormick commented Jun 3, 2025 •

Copy link
Copy Markdown
Collaborator

TAPHandle has been developed by the SSC XMM-Newton team in Strasbourg. The correct label is: Strasbourg Observatory - SSC XMM-Newton

@lmichel

Is there an appropriate acronym that can be used for this?

The other organizations in this column are quite short as they are all acronyms. (This page really needs a glossary for these, but that's another issue.)

@lmichel

lmichel commented Jun 3, 2025

Copy link
Copy Markdown

ObAS

@gmantele gmantele mentioned this pull request Jun 12, 2025
@gmantele

Copy link
Copy Markdown
Member

This CI failed because the SFTP configuration is incorrect for the preview of a branch. It has been correctly configured for the main branch but not for the others.

@molinaro-m , @JeremyMcCormick , @msdemlei Is it correct if I copy-paste the configuration (username, password) from deploy-main.yaml into deploy-version.yaml?

Of course, I won't change the target directory.

Its author (Laurent Michel) suggested "Strasbourg Observatory - SSC XMM-Newton". However, a shorter version was preferred: "ObAS".
Also write the complete affiliation. It is an important product from SSC XMM-Newton and is made by the Observatory of Strasbourg (not CDS)....so, both deserve to be equally highlighted.
@JeremyMcCormick
JeremyMcCormick dismissed their stale review September 2, 2026 00:52

dimissing stale review

@molinaro-m

Copy link
Copy Markdown
Member

Hi @gmantele, @msdemlei, @JeremyMcCormick , adding @szautkin.

I think the issue is this branch being on a fork.
The error I got, after updating the staging workflow looks like it is not able to reach the repo secrets (because I suppose it works on a different repo, Markus's one).

If I remember correctly, the workflows were set up to work from branches on the ivoa/ivoa-web repo.

Not sure if this is actually the issue, nor how to solve it, besides moving the branch into this repo.

@gmantele

gmantele commented Sep 2, 2026

Copy link
Copy Markdown
Member

One could try to move the branch in the ivoa organization and let's see.
@msdemlei , since it is your PullRequest, do you want to?

@msdemlei

msdemlei commented Sep 2, 2026 via email

Copy link
Copy Markdown
Contributor Author

@molinaro-m

Copy link
Copy Markdown
Member

OK, I take the risk on me, since it's definitely minimal, a change on 1 line, not affecting other stuff.
I merge it.

Anyway, yes, the idea is to have a staging preview before putting things in production, that's why we are locked in the PRs from branches on the primary origin repo.

@molinaro-m
molinaro-m merged commit c93cfbc into ivoa:main Sep 2, 2026
1 check failed
@molinaro-m

Copy link
Copy Markdown
Member

merged
visible on main site
the modified staging setup needs other tests

@msdemlei

msdemlei commented Sep 2, 2026 via email

Copy link
Copy Markdown
Contributor Author

@gmantele

gmantele commented Sep 2, 2026

Copy link
Copy Markdown
Member

Hm. I'm not really sure what "upload the branch version" is supposed to do, but frankly I think we shouldn't do uploads from within a PR's CI. To my instinct, there's nothing wrong with building the page and perhaps run a few tests on the result (perhaps "new broken links", although I'd really limit that to internal links), but that's it. Was the idea to have a preview rendering somewhere?

The idea of this CI is exactly that: to build the website and to upload it somewhere so that we can preview the result before merging and publishing it for everyone.

But yes, I totally agree that this PR was a lot of interactions just to change a simple label and URL.


the modified staging setup needs other tests

I agree with @molinaro-m , we need to test the staging setup in order to avoid a so long process the next time.


Ok, if that's the way we want it in the repo, let's document it somewhere (in the README, I suppose, which then should also say where the staging server is; while doing that, updating the current banner ("re-write the current") could be updated, too).

That is actually written in the CONTRIBUTING.md file. Or would you mean to write something else?

@msdemlei

msdemlei commented Sep 2, 2026 via email

Copy link
Copy Markdown
Contributor Author

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.

5 participants