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

Voevent 2.1 pr after RFC #21

Merged
merged 31 commits into from
Jan 28, 2025

Conversation

BaptisteCecconi
Copy link
Collaborator

No description provided.

Copy link

@gmantele gmantele left a comment

Choose a reason for hiding this comment

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

This PullRequest contains a lot of modifications that I did not look into details. But here are my three comments:

  1. A change request: a conflict has not been properly resolved ; it has left a trace with the famous <<<< HEAD string.
  2. I can generate the PDF, but not the SVG images inserted into the PDF, although I have Inkscape installed on my machine. Is it normal?
  3. The RFC is not yet finished. A lot of WG (including DAL....shame on me) and IG have not yet commented or voted. Maybe you would want to wait for all comments before merging this PR...

VOEvent.tex Outdated Show resolved Hide resolved
@BaptisteCecconi
Copy link
Collaborator Author

This PullRequest contains a lot of modifications that I did not look into details. But here are my three comments:

  1. A change request: a conflict has not been properly resolved ; it has left a trace with the famous <<<< HEAD string.

Thanks, I missed this one. This is fixed in 7bc253e

  1. I can generate the PDF, but not the SVG images inserted into the PDF, although I have Inkscape installed on my machine. Is it normal?

No idea. Note that I updated the ivoatex submodule. Could this be linked?

  1. The RFC is not yet finished. A lot of WG (including DAL....shame on me) and IG have not yet commented or voted. Maybe you would want to wait for all comments before merging this PR...

I included most of the comments from @mcdittmar and @msdemlei, when I considered them relevant, and when @msdemlei didn't say he was willing to propose a PR. To ease the process of @msdemlei to propose his updates, could we merge this PR and then start from the new content?

Copy link

@gmantele gmantele left a comment

Choose a reason for hiding this comment

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

Looks fine to me.

@gmantele
Copy link

This PullRequest contains a lot of modifications that I did not look into details. But here are my three comments:

  1. A change request: a conflict has not been properly resolved ; it has left a trace with the famous <<<< HEAD string.

Thanks, I missed this one. This is fixed in 7bc253e

You're welcome 😉

  1. I can generate the PDF, but not the SVG images inserted into the PDF, although I have Inkscape installed on my machine. Is it normal?

No idea. Note that I updated the ivoatex submodule. Could this be linked?

No idea either. I've just make this comment as a warning...something to take a look at, otherwise the version on IVOA-Doc repo will not look good.

  1. The RFC is not yet finished. A lot of WG (including DAL....shame on me) and IG have not yet commented or voted. Maybe you would want to wait for all comments before merging this PR...

I included most of the comments from @mcdittmar and @msdemlei, when I considered them relevant, and when @msdemlei didn't say he was willing to propose a PR. To ease the process of @msdemlei to propose his updates, could we merge this PR and then start from the new content?

Fine with me. Again, it was just a general comment, nothing blocking for me 😉

@gmantele
Copy link

I let you merge whenever you think it is the right moment.

@msdemlei
Copy link
Contributor

msdemlei commented Jan 16, 2025 via email

@gmantele
Copy link

On Wed, Jan 15, 2025 at 07:14:42AM -0800, Baptiste Cecconi wrote: > 2. I can generate the PDF, but not the SVG images inserted into the PDF, although I have Inkscape installed on my machine. Is it normal? No idea. Note that I updated the ivoatex submodule. Could this be linked?
You mean the role diagram? What happens if you try? Updating the submodule is almost always a good thing. I keep trying to make the submodules thing work better (does make update work for you?). If nothing else helps, try (cd ivoatex; git checkout master; git pull) If SVG generation still does not work, I'd be interested in the error messages. Note, however, that the preferred svg -> pdf renderer now is now rsvg-convert from librsvg2-bin, which is a lot more compact and is less volatile in the CLI than inkscape is.

My fault: I forgot to recursively clone this repo resulting in no ivoatex at all. Besides, rsvg-convert was not installed on this machine (my comment about Inkscape is because of the generated fallback PDF stating that no Inkscape was found although it was on my machine ; maybe this error message should be updated). It seems to work nicely now. Thanks for forcing me to look into this one more time. It should probably be noted that make clean does not remove role-diagram.pdf ; I had to remove it manually after my first attempt without rsvg-convert in order to have it re-generated properly.

It is all good for me now.

@BaptisteCecconi BaptisteCecconi merged commit 238750f into ivoa-std:master Jan 28, 2025
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