Skip to content

Port to ivoatex#22

Open
glemson wants to merge 10 commits intomasterfrom
port-to-ivoatex
Open

Port to ivoatex#22
glemson wants to merge 10 commits intomasterfrom
port-to-ivoatex

Conversation

@glemson
Copy link
Collaborator

@glemson glemson commented Apr 21, 2021

blind translation of md to tex. combined into one file following ivoatex structure.
Added ivoatex as submodule and copied Makefile.
Was able to build to pdf.

@glemson glemson requested review from mcdittmar and olaurino April 21, 2021 18:00
@olaurino
Copy link
Collaborator

Is it an ivoatex policy to include the PDF in version control? It being in a binary format, and one that needs to be generated, it's usually not a good idea to include it in version control. But again, I am not sure if there are any established practices in this context that call for it.

@tomdonaldson
Copy link
Collaborator

Is it an ivoatex policy to include the PDF in version control? It being in a binary format, and one that needs to be generated, it's usually not a good idea to include it in version control. But again, I am not sure if there are any established practices in this context that call for it.

I think the pdf should not be included in version control. It is binary and buildable from the controlled components, so as you say, we should not be saving a copy in version control. I don't think any of the other standards are storing the pdfs in version control.

@olaurino
Copy link
Collaborator

@tomdonaldson if that is the case, then the pdf should be removed with a rebase/force push, because otherwise it would be kept in version control, even if a new commit deletes it.

@glemson
Copy link
Collaborator Author

glemson commented Apr 21, 2021

yes, my bad. I did not add .pdf file in the .gitignore. THIS IS WRONG

Don't know how the pdf ended up there for the .gitignore did actually contain it.
Wonder if I inadvertently add-ed it explicitly.

@glemson
Copy link
Collaborator Author

glemson commented Apr 21, 2021

btw, @tomdonaldson are all standards supposed to be under github/ivoa-std?
And can only wg chairs able to define a new repo there?
If so then we are here not working on a standard (yet), but we can pretend I suppose.
I am by no means an expert on github but am happy to try hard removing the pdf.

@glemson
Copy link
Collaborator Author

glemson commented Apr 21, 2021

ok, someone smarter than me re git(hub) should probably do this.
I tried following some online suggestions, but clearly did not do the right thing as the pdf is still there.

CORRECTION with help of former CfA person I think I was able to remove the pdf file.
branch is now only one commit long again. please have a look

@tomdonaldson
Copy link
Collaborator

btw, @tomdonaldson are all standards supposed to be under github/ivoa-std?
And can only wg chairs able to define a new repo there?
If so then we are here not working on a standard (yet), but we can pretend I suppose.
I am by no means an expert on github but am happy to try hard removing the pdf.

@glemson Yes, standards should belong in ivoa-std. I'm not sure if there is a criteria for when things that might become a standard belong there.

I don't think the WG chairs can create a new repository. That is probably something for the TCG chair, but I notice there are some other owners of the ivoa-std, so they probably can as well (https://github.com/orgs/ivoa-std/people?query=role%3Aowner), though I don't know if they are supposed to. This might be a good question for the IVOA #github-help Slack channel as well.

@msdemlei
Copy link
Collaborator

msdemlei commented Apr 22, 2021 via email

@glemson
Copy link
Collaborator Author

glemson commented Apr 22, 2021

Actually I think one could make the case for our standards development to include the built products in the repo at certain points of its life cycle. For in contrast to say compiled class files in a software dev project, here the readable product happens to be the pdf file. If I want to evaluate the latest PR I may be doing this from a location that does not allow me to first build the result, then view it. E.g. when accessing github from a tablet. Having the PDF directly available on the site would I think be useful. One might allowthis only to a PR, i.e. not every push, but I can see a use.

@olaurino
Copy link
Collaborator

@glemson this is what build services (aka continuous integration) can do. The repository can have tags and releases with associated products. That's what I used to do with this document btw. At each PR the document was built automatically, and I think people are doing that with ivoa-tex as well. I'm not going to make blanket statements about never put a built product under version control, but it's rare.

@glemson
Copy link
Collaborator Author

glemson commented Apr 22, 2021

ok. if ivoatex/ivoa-std etc is set up like that then my case is moot.
Is it @msdemlei ?

@msdemlei
Copy link
Collaborator

msdemlei commented Apr 23, 2021 via email

@mcdittmar
Copy link
Collaborator

mcdittmar commented Apr 23, 2021 via email

@glemson
Copy link
Collaborator Author

glemson commented Apr 23, 2021 via email

@glemson
Copy link
Collaborator Author

glemson commented Apr 25, 2021 via email

@gmantele
Copy link
Member

Hi @glemson ,

Apparently, the GitHub Workflow build.yml did not complete and so, there is no PDF artefact accessible.

The Workflow failed at the step where it checks whether the PDF exists or not (see the detailed logs at https://github.com/ivoa/mapping-vodml/runs/2432401684?check_suite_focus=true). As far as I can see, the PDF MappingVODML.pdf was successfully created but the test -f failed for some reason.

Here is my quick guess. This test is done on 2 files: MappingVODML.pdf and MappingVODML.bbl. Since the PDF apparently exists, maybe the 2nd one (MappingVODML.bbl) does not. Hence the "false error". If so, it would be easy to test:

  • comment the test test -f MappingVODML.bbl in build.yml and preview.yml, and see if it works
  • or, probably cleaner, see why this MappingVODML.bbl was not generated.

When this Workflow will work, you should see a green tick symbol beside the last commit. Clicking on this green tick (or otherwise the red cross) will bring you to the summary and detailed logs of the corresponding Workflow's run. If the Workflow was a success, the PDF artefact should be accessible in this page ; you should see an 'Artefacts' section like in here in the ADQL repo.

Anyway, the link you have in the README.md will work only when the GitHub Workflow preview.yml will run. This latter runs only when the branch will be merged. At that moment, you will see on the right part of the Web page a 'Releases' item. This will be a draft release in which the pre-built PDF will be publicly available. That's where the link in the README.md points to. In conclusion, it is normal that for the moment the link does not work. 2 conditions have to be reached for that: 1/ make the build.yml successfully works (and modify preview.yml accordingly if needed), 2/ merge the branch with master.

@gmantele
Copy link
Member

On the other hand, you can have pre-built PDFs for use on locked-down platforms (or whatever) thanks to github actions integration contributed by Grégory (which regrettably hasn't made it to ivoatexDoc -- help is welcome!)

I definitely have to do that....it is still in my TODO list. Sorry for the delay :(

@glemson
Copy link
Collaborator Author

glemson commented May 10, 2021

Thanks all for your help and comments.
Yesterday I merged changes Omar had a PR for in the original, markdown version.
I merged its changes into the current branch and ported the markdown updates to ivoatex as well.
This is now pushed and it seems the automated compilation works now.
I do not claim the port to be done, maybe citations need updating, text sure needs more review.
But please @olaurino and @mcdittmar if you can review the current state, it would be nice if we can get this merged to master for further writing.

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.

6 participants