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

ISIS Docs build fix for incorrect path and -WEBHELP issues #4511

Merged
merged 7 commits into from
Jun 9, 2021

Conversation

KrisBecker
Copy link
Contributor

When Building ISIS documentation, the dot command that builds graphics for Programmer and Developer documentation is not found.

Also, applications built in this environment cannot get web documentation due to a path error.

Description

$ISISROOT/doc/ path specification is invalid in UserInterface. An "s" was added to "doc".

Also, the Graphviz package was added to the environment files to provide the dot command for Doxygen builds.

Related Issue

This PR fixes #4510

Motivation and Context

Corrects localized access to ISIS documentation

How Has This Been Tested?

The fix was tested in my build environment.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation change (update to the documentation; no code change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have read and agree to abide by the Code of Conduct
  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have added myself to the .zenodo.json document.
  • I have added any user impacting changes to the CHANGELOG.md document.

Licensing

This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words:

This work is free and unencumbered software released into the public domain. In jurisdictions that recognize copyright laws, the author or authors of this software dedicate any and all copyright interest in the software to the public domain.

  • I dedicate any and all copyright interest in this software to the public domain. I make this dedication for the benefit of the public at large and to the detriment of my heirs and successors. I intend this dedication to be an overt act of relinquishment in perpetuity of all present and future rights to this software under copyright law.

Update my ISIS3 fork from Astrog/ISIS3...DONE!
Sync with USGS/Astro dev branch
Update my fork with USGS latest. DONE.
Merge current Astro/ISIS3 changes (2021-06-03)
Update my fork to Astro/ISIS3...DONE.
When building ISIS documentation, the dot command is not found. This dependancy is in the graphiviz package, which is added to the environment files.
The hardcoded path to ISIS docs was incorrect. The path to docmentation is $ISISROOT/docs - the s was missing in the path spec.
Copy link
Contributor

@krlberry krlberry left a comment

Choose a reason for hiding this comment

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

This all looks good to me!

@KrisBecker
Copy link
Contributor Author

Ok, so I am not sure what happens now.

Will someone just merge this into dev?

I see the Jenkins test for this PR is failing but the 10 errors in the build seem completely unrelated to this PR. All other PRs I checked have the same errors, so no PR Jenkins tests are passing right now? Are passing Jenkins tests a pre-requisite for a merge?

@jessemapel jessemapel merged commit 820a259 into DOI-USGS:dev Jun 9, 2021
@jessemapel
Copy link
Contributor

We are having a few issues with our Jenkins builds at the moment that we're working to fix. This is also missing a CHANGELOG entry, but we'll add that to the PRs for the bug fix release we're prepping right now.

@KrisBecker
Copy link
Contributor Author

Ok, thanks. Hope you can indulge a few more questions.

I notice that my PR has every PR I used to update/sync my Astrogeology/ISIS3 fork. What is the recommended way to only see the relevant commits and and not the sync PRs on my fork? For example, I have another PR coming that depends on this PR. I want now to sync my GitHub ISIS3 fork and branch off that state of dev. What is the best way to do this and prevent the sync PR from following me around?

Thanks for the CHANGELOG update. So every PR requires a change log entry?

Finally, I don't appear to have merge permissions (I am not asking for it either). Following the process as outlined in the contributing docs and howtos, it seems after the PR is submitted and reviewed, it is not clear to me how to get the PR merged and closed out. Is there someone at Astro who is the designated external Committer for a particular time frame or is the decision to merge mostly adhoc once comments/reviews cease? Is there a customary review/comment period? I am trying to understand what are the steps and average expected time frame to complete a PR.

Thanks in advance...

@jessemapel
Copy link
Contributor

The best way to sync your changes with upstream is to rebase them using git rebase.

Generally once a PR is approved it can be merged as long as it's been open for more than a handful of hours. The submitter can merge their own PR after it has been approved by another, the reviewer may or may-not merge it. Generally if a reviewer thinks a PR looks good, but wants another person to review they should indicate that in their review.

@KrisBecker
Copy link
Contributor Author

Ok, I finally found something that works. Here are the instructions I used to fix the commits ahead state created by using PRs to sync my ISIS3 fork:

git checkout dev
git remote add upstream https://github.com/USGS-Astrogeology/ISIS3.git
git fetch upstream
git rebase upstream/dev
git push -f origin dev

The -f is important to force the update.

See also this post.

jessemapel pushed a commit that referenced this pull request Jul 31, 2021
* Add missing dot command from graphviz package

When building ISIS documentation, the dot command is not found. This dependancy is in the graphiviz package, which is added to the environment files.

* Fixed path to ISIS docs for -WEBHELP

The hardcoded path to ISIS docs was incorrect. The path to docmentation is $ISISROOT/docs - the s was missing in the path spec.
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.

Build Problems with ISIS Docs and -WEBHELP Usage
3 participants