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

Changes to get Metanorma to build locally. #355

Merged
merged 1 commit into from
Sep 13, 2023
Merged

Changes to get Metanorma to build locally. #355

merged 1 commit into from
Sep 13, 2023

Conversation

gbuehler
Copy link
Contributor

I am finding all kinds of silly ways to break Metanorma.

So far, I have had to

  1. Take single quotes from table titles (replaced with backticks) as they put extraneous text in the HTML file. [NOTE: This is only an issue in the front material.]

Screenshot 2023-09-12 at 5.07.06 PM.png

In clause_0_front_material.adoc, this works:


[[table_core_resources]]
.Requirements class Core
[cols="27,25,10,10,28",options="header"]

This does not:


[[table_core_resources]]
.Requirements class 'Core'
[cols="27,25,10,10,28",options="header"]

However, in clause_9_security-considerations.adoc, this works:


[[table_core_safe_operations_security]]
.Requirements class 'Core' - Overview of core operations and returned sensitive information
[cols="20,30,10,40",options="header"]

  1. In an included requirement (REQ_test-process.adoc), a cross-reference to a future included item's (REC_test-process.adoc) bookmark doesn't seem to work.

In REQ_test-process.adoc, this works:


If a server implementing the OGC API - Processes - Part 1: Core is tested using CITE tests, the server SHALL offer at least one testable process. Please refer to
// <<rec_core_test-process>>
for further guidance.


This does not:


If a server implementing the OGC API - Processes - Part 1: Core is tested using CITE tests, the server SHALL offer at least one testable process. Please refer to <<rec_core_test-process>> for further guidance.

NOTE: I have not figured out how to actually fix this yet!

  1. It seems that explicit ordering of ordered lists with subitems (nested) in Abstract Test items does not work

In ATS_landingpage-success.adoc, this works:


test-method::
+

. Validate that a document was returned with an HTTP status code or 200.

. Validate the landing page for all supported media types using the resources and tests identified in <>

. For formats that require manual inspection, perform the following:

.. Validate that the landing page includes a "service-desc" and/or "service-doc" link to an API Definition.

.. Validate that the landing page includes a "http://www.opengis.net/def/rel/ogc/1.0/conformance" link to the conformance class declaration.

.. Validate that the landing page includes a "http://www.opengis.net/def/rel/ogc/1.0/processes" link to the list of processes.

--

This does not:


--

  1. Validate that a document was returned with an HTTP status code or 200.

  2. Validate the landing page for all supported media types using the resources and tests identified in <>

  3. For formats that require manual inspection, perform the following:

    a. Validate that the landing page includes a "service-desc" and/or "service-doc" link to an API Definition.

    b. Validate that the landing page includes a "http://www.opengis.net/def/rel/ogc/1.0/conformance" link to the conformance class declaration.

    c. Validate that the landing page includes a "http://www.opengis.net/def/rel/ogc/1.0/processes" link to the list of processes.

--

  1. This didn't stop the build, but it was displaying incorrectly:

Screenshot 2023-09-12 at 5.26.45 PM.png

This does not work:


[[ats_ogc-process-description]][conformance_class]

This works:


[[ats_ogc-process-description]]

[conformance_class]

@jerstlouis
Copy link
Member

jerstlouis commented Sep 12, 2023

Thanks a lot @gbuehler for doing all the hard work of debugging this.

One of the issues you mentioned seemed like a similar issue to what we faced in DGGS trying to include code blocks inside requirement parts ( metanorma/metanorma#312 ).

Could you / we please edit this commit to avoid adding the generated PDF / HTML to the repository?

We should probably also add this to a .gitignore to avoid this from accidentally happening in the future.
I think *.pdf should perhaps even be in the default .gitignore of all default OGC templates :)

@pvretano pvretano merged commit e9faf6e into master Sep 13, 2023
1 check passed
@jerstlouis
Copy link
Member

jerstlouis commented Sep 13, 2023

Merging this as-is added the generated PDF into the repository (which preserves all history unless we force push an amended history), forever increasing the size of the repository, by 5 Megabytes, and slowing down all git actions that need to clone the repository, and every single new clone of the repository :(

It also will irresistibly tempt everyone building locally to commit their local changes to the PDF / HTML to commit their own version, each time adding an extra 5 megabytes.

We really need a hard OGC rule about not adding generated files (PDF document outputs in particular) to Git repositories. Please :)

@pvretano
Copy link
Contributor

Doh! @jerstlouis forgot about it even though @gbuehler menitoned it in his email. I'll just remove the PDF and HTML. The entire point of the thread was to auto generate these anyway. OK?

@jerstlouis
Copy link
Member

jerstlouis commented Sep 13, 2023

@pvretano Ideally you would do this with a git commit --amend followed by a git push --force as opposed to a new commit, so as to delete it from the history. Though not sure how that works with a "merge" commit, since it may introduce the original commit anyhow. (it's also more complicated since this is earlier in the history now).

@pvretano
Copy link
Contributor

@jerstlouis can you do it? I might screw it up because I have never had to amend a merge commit.

@jerstlouis
Copy link
Member

jerstlouis commented Sep 13, 2023

@pvretano Sure, I will do it but it means that everyone (who had fetched / pulled since) should then:

  • git stash any unstaged (not added / not committed) local work area changes
  • git fetch
  • git reset --hard origin/master
  • git stash pop (if any stashed changes to restore)

Otherwise someone might re-introduce the commit again by merging with the older version.

I passionately hate merge commits about as much as I hate generated PDFs in Git repositories :)

Our internal workflow on the main branch is always:

  • git fetch (to get latest)
  • git rebase origin/master (to rebase in case there's anything new on master since when we committed something)
  • git push

or to just update remote changes:

  • git fetch
  • git merge --ff-only origin/master

or using git pull but avoiding a merge commit:

  • git pull --rebase

I avoid merge commits like the plague (I never ever use git merge or git pull by themselves, and never ever click the GitHub / GitLab Merge button) . Merge commits are particularly nasty with trying to use git bisect to track commits breaking regressions, but also to just easily understand the output of git log.

Although things might have improvement more recently:

https://discourse.llvm.org/t/rfc-revisiting-linear-history-vs-merge-commits/64873#:~:text=A%20single%20point%20to%20revert,atomically%2C%20while%20having%20separate%20history.

https://stackoverflow.com/questions/17267816/git-bisect-with-merged-commits

And opinions differ:

https://news.ycombinator.com/item?id=12582432

I am definitely terrified of merge commits :)

@jerstlouis
Copy link
Member

@pvretano Done.

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