Skip to content
This repository has been archived by the owner on Oct 17, 2019. It is now read-only.

Updates Needed by IG Publisher #109

Merged
merged 6 commits into from Oct 2, 2018
Merged

Updates Needed by IG Publisher #109

merged 6 commits into from Oct 2, 2018

Conversation

cmoesel
Copy link
Member

@cmoesel cmoesel commented Oct 1, 2018

This contains the following changes in order to keep up w/ IG publisher process & requirements:

  • Fixed unquoted href value and unterminated HTML <head> tag. (Oops).
  • Added a <!--status-bar--> comment and igtool="footer" attribute per Grahame's instructions (here)
  • Host the modeldoc in an iFrame rather than launching a new window (to meet requirement that certain elements be on every page)
  • Update the IG Publisher to the daily build from Oct 1

@cmoesel
Copy link
Member Author

cmoesel commented Oct 1, 2018

Note that when you run the IG publisher, if you look at the QA file (yarn ig:qa), you'll still see a bunch of errors about missing <!-- status-bar --> and footers. Those are on all of the modeldoc pages that are in the frameset. We shouldn't put the status bar and footers in them because they'd look stupid -- and the status/footer is already in the page that encloses the modeldoc in an iframe.

@kjmahalingam
Copy link
Contributor

In general, all of this looks good. I do have a couple of questions about errors that appear in the console of the page when using it, though.

On every page of the IG when I run it, I get this error:

ig_cors_error

This seems to be related to CORS. Is this a typical issue when running locally, or is this a new problem?

Additionally, on the model doc page, all of these errors are shown:
ig_modeldoc_errors

None of these seem to affect anything from what I can tell, but I want to ensure that these aren't an issue, and see if they need to be resolved.

@kjmahalingam
Copy link
Contributor

kjmahalingam commented Oct 1, 2018

I just tested again with master and was able to verify that the CORS error existed there as well. So I'm assuming that shouldn't be an issue. The other console errors, however, I still have questions about.

Additionally, when I ran on this branch, I got a result from the publisher of:
Errors: 0 Warnings: 30 Info: 0 (00:03:44.0935sec)

But when I ran on master, I got a result from the publisher of:
Errors: 0 Warnings: 0 Info: 0 (00:05:42.0072sec)

Should we be concerned about the warnings that appear using this branch? Or are those only related to the issue you noted above?

@cmoesel
Copy link
Member Author

cmoesel commented Oct 1, 2018

Right. Some console errors are due to running on the file system instead of HTTP. If you host it using an HTTP server, they'll go away. For example, on mac:

cd ./out/fhir/guide/output/
python -m SimpleHTTPServer 8000

Browsing to http://localhost:8000/index.html, I don't get any console errors:

image

Going to the modeldoc, however, I still get some:

image

The first two are issues w/ the modeldoc itself (outside the scope of this PR). The third may be introduced by this PR (since top is now referring to the page that contains the iframe) -- but as far as I can tell, it doesn't seem to affect anything. It too, however, would have to be fixed in the modeldoc repo.

@cmoesel
Copy link
Member Author

cmoesel commented Oct 1, 2018

I'll look at those warnings. I thought we got those before too, but maybe not. If they're new, it's likely due to the igpublisher jar flagging things it didn't used to flag. I'll take a look and report back.

@cmoesel
Copy link
Member Author

cmoesel commented Oct 1, 2018

OK. Some of these are valid warnings, but caused by issues in the CIMPL definitions (unrelated to this PR). It must be that the old IG Publisher jar didn't catch these.

For example, this is an incorrect display value:

image

Some others, however, seem invalid -- but again, aren't really our issue. For example, this is flagged as a mismatch, but it looks fine to me:

image

Those are the only two types of warnings encountered (valid warnings about terminology mismatches and seemingly invalid warnings about terminology mismatches).

Copy link
Contributor

@kjmahalingam kjmahalingam left a comment

Choose a reason for hiding this comment

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

Thank you for the clarification. I am approving and will merge, given that any issues we do see are out of the scope of this PR.

@kjmahalingam kjmahalingam merged commit 78b0d32 into master Oct 2, 2018
@cmoesel
Copy link
Member Author

cmoesel commented Oct 2, 2018

I'm working on updates to the modeldoc code that will address the remaining issues.

@cmoesel cmoesel deleted the igpub-update branch October 31, 2018 13:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants