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

select repo key package based on grains['os'] #41

Conversation

jdkelleher
Copy link
Contributor

Added if/else based on grains['os'] around common keyring packages to pillar.example. Simple change, but makes the example more usable and should promote pulls with additional distribution details.

@aboe76
Copy link
Member

aboe76 commented Apr 30, 2019

@jdkelleher shouldn't this be mapped within map.jinja

@daks
Copy link
Member

daks commented Apr 30, 2019

Hello @jdkelleher, I don't understand what this code is for. Can you explain us your problem or difficulties using thi formula?

@jdkelleher
Copy link
Contributor Author

@jdkelleher shouldn't this be mapped within map.jinja

Yes, I guess that's where defaults belong. I can rework this to add defaults for Ubuntu and Linux Mint to the 'map.jinja' file and resubmit.

@aboe76
Copy link
Member

aboe76 commented May 15, 2019

@jdkelleher please rework and resubmit if you don't mind.

Copy link
Member

@n-rodriguez n-rodriguez left a comment

Choose a reason for hiding this comment

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

sounds good!

@jdkelleher
Copy link
Contributor Author

This is not needed - closing pull request. I will be submitting map.jina updates shortly to add Ubuntu defaults.

@jdkelleher jdkelleher closed this May 23, 2019
@myii
Copy link
Member

myii commented May 23, 2019

@jdkelleher You're welcome to introduce the changes in this PR, using the same branch. Rebasing either from your side or we can do that when merging (including squashing). One benefit of doing that is so that the whole discussion is kept together in one place, including the fact that there has been one approving review of this PR.

@n-rodriguez
Copy link
Member

n-rodriguez commented May 23, 2019

including the fact that there has been one approving review of this PR.

yeah...the idea is good, but map.jinja is a better solution... and the new map.jinja is actually even better, but not implemented yet :/

javierbertoli added a commit to netmanagers/apt-formula that referenced this pull request May 27, 2019
* Use the correct keyring for the OS (a refactor of the changes suggested in saltstack-formulas#41)
@jdkelleher jdkelleher reopened this May 27, 2019
@jdkelleher jdkelleher closed this May 27, 2019
@jdkelleher jdkelleher deleted the enhance-pillar.example branch May 27, 2019 15:55
@jdkelleher jdkelleher mentioned this pull request May 27, 2019
javierbertoli added a commit to netmanagers/apt-formula that referenced this pull request May 27, 2019
* Use the correct keyring for the OS (a refactor of the changes suggested in saltstack-formulas#41)
myii pushed a commit to myii/apt-formula that referenced this pull request May 27, 2019
## [1.7.1](v1.7.0...v1.7.1) (2019-05-27)

### Bug Fixes

* **map.jinja+tests:** add correct keyring for the OS ([0ff48e1](0ff48e1)), closes [saltstack-formulas#41](https://github.com/myii/apt-formula/issues/41)

### Continuous Integration

* **kitchen+travis:** update testing environment ([3fa2a58](3fa2a58))
saltstack-formulas-travis pushed a commit that referenced this pull request May 28, 2019
## [0.8.1](v0.8.0...v0.8.1) (2019-05-28)

### Bug Fixes

* **map.jinja:** typo and lookup order ([eda8517](eda8517))
* **map.jinja+tests:** add correct keyring for the OS ([0ff48e1](0ff48e1)), closes [#41](#41)

### Code Refactoring

* Merge branch 'upstream/master' ([1496eed](1496eed))

### Continuous Integration

* **kitchen+travis:** update testing environment ([3fa2a58](3fa2a58))

### Documentation

* **kitchen:** update documentation ([530e22c](530e22c))
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.

5 participants