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

Plenary templates readme fixes and improvements #324

Merged
merged 2 commits into from
Dec 13, 2019

Conversation

jouvin
Copy link
Contributor

@jouvin jouvin commented May 25, 2018

  • Fix a syntax error (missing ;)
  • Update the template name to loadpath to be consistent with the Aquilon PR in progress
  • Slightly different (improved 😄 ) version of the loadpath template
    • Order consistent with what was done in SCDB (which seems more sensible in case a template in one template library component is overriden in another template library components).
    • Suggestion to move the OS-related loadpath definition to the config/os/... expected by Aquilon as it requires the OS version which is normally defined there.

- Use archetype/loadpath instead archetype/base
- Missing ;
@jouvin jouvin requested a review from jrha May 25, 2018 17:09
@jrha
Copy link
Member

jrha commented May 29, 2018

I will wait for the aquilon PR to be finalised before looking at this too hard.

Copy link
Member

@jrha jrha left a comment

Choose a reason for hiding this comment

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

LGTM

src/scripts/plenary_template_library/README.md Outdated Show resolved Hide resolved
src/scripts/plenary_template_library/README.md Outdated Show resolved Hide resolved
@jrha jrha added this to the 19.12 milestone Dec 13, 2019
@jrha
Copy link
Member

jrha commented Dec 13, 2019

Updated to match code which was eventually merged into aquilon and documented config option that needs to be set.

@jrha jrha merged commit 7a9bf89 into quattor:master Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants