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

feat(map): update to v5 map.jinja #17

Merged

Conversation

baby-gnu
Copy link
Contributor

@baby-gnu baby-gnu commented Feb 25, 2021

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

Yes.

BREAKING CHANGE: map.jinja now export a generic mapdata variable

BREAKING CHANGE: The parameters per grains are now under java/parameters/

Related issues and/or pull requests

Describe the changes you're proposing

map.jinja is now generic, the post-processing is now done in java/post-map.jinja.

The java-formula requires a dedicated map.jinja configuration to load values for the kernel grains.

Pillar / config required to test the proposed changes

Debug log showing how the proposed changes work

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

@baby-gnu baby-gnu requested review from noelmcloughlin and a team as code owners February 25, 2021 13:12
@baby-gnu baby-gnu force-pushed the feature/v5-map.jinja branch from 409d2ee to 6b97453 Compare February 25, 2021 13:19
`map.jinja` is now generic, the post-processing is now done in
`post-map.jinja`.

The `java-formula` requires a dedicated `map.jinja` configuration to
load values for the `kernel` grains.

BREAKING CHANGE: `map.jinja` now export a generic `mapdata` variable

BREAKING CHANGE: The parameters per grains are now under `java/parameters/`
We need to update the reference files to include the `map.jinja`
sources configuration.
@baby-gnu baby-gnu force-pushed the feature/v5-map.jinja branch from 6b97453 to 80a3f8f Compare February 25, 2021 13:51
@myii
Copy link
Member

myii commented Feb 25, 2021

Nice finale for this series of PRs, @baby-gnu! All good from my end but I'll just leave time for @noelmcloughlin, who is the main code owner here.

@baby-gnu
Copy link
Contributor Author

It looks like I changed the order in which the values are loaded from the YAML files:

  • previously
    1. defaults.yaml
    2. os_family
    3. osarch
    4. kernel
    5. salt['config.get']("java:lookup")
    6. salt['config.get']("java")
  • this PR
    1. defaults.yaml
    2. osarch
    3. os_family
    4. os
    5. osfinger
    6. kernel
    7. salt['config.get']("java:lookup")
    8. salt['config.get']("java")
    9. id

I can modify parameters/map_jinja.yaml to use the same order as before or maybe add a BREAKING CHANGE to say that the ordering is now changed?

Copy link
Member

@noelmcloughlin noelmcloughlin left a comment

Choose a reason for hiding this comment

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

Looks good to me. thanks.

@noelmcloughlin
Copy link
Member

It looks like I changed the order in which the values are loaded from the YAML files:

Only if you are aware it causes a problem I guess.
I think this repo mainly manages archives so ordering might not be improtant for archives.

@noelmcloughlin noelmcloughlin merged commit 0dc65d2 into saltstack-formulas:master Feb 25, 2021
@saltstack-formulas-travis

🎉 This PR is included in version 2.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@baby-gnu baby-gnu deleted the feature/v5-map.jinja branch February 25, 2021 15:14
myii added a commit to myii/ssf-formula that referenced this pull request Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants