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

Import YAML with context #56

Merged
merged 2 commits into from
Aug 12, 2021
Merged

Conversation

Lucianovici
Copy link
Contributor

Import with context to avoid errors like:

[CRITICAL] Rendering SLS ... failed: Jinja variable 'grains' is undefined
[CRITICAL] Rendering SLS ... failed: Jinja variable 'salt' is undefined

@noelmcloughlin
Copy link
Member

@Lucianovici could you update both commit messages to comply with conventional commit standard.
see https://www.conventionalcommits.org/en/v1.0.0/#summary CI is failing.

@Lucianovici
Copy link
Contributor Author

Lucianovici commented Aug 10, 2021

@noelmcloughlin Is it important to fix this?

The solution of renaming pushed commit messages is not ideal (forcing to push) - is there another way I am not aware of?

@noelmcloughlin
Copy link
Member

Is it important to fix this?

Yes. All automation based on conventional commits (widely used today instead of unconventional commits) breaks. Someone would have to change those commit messages to get CI and Semantic Release working.

@Lucianovici
Copy link
Contributor Author

@noelmcloughlin
Copy link
Member

I'm not maintainer, just passing by, so its extra workload. I think its possible, I've never done that, I could try I guess.

node-formula/CODEOWNERS

Lines 11 to 12 in 79081c9

# SECTION: Owner(s) for specific directories
# FILE PATTERN OWNER(S)

@noelmcloughlin noelmcloughlin merged commit 7e56f69 into saltstack-formulas:master Aug 12, 2021
@noelmcloughlin
Copy link
Member

Okay, that did not work, I'll try to remember to open a new PR to fix your commit messages. Please use conventional commits in future, its considered impolite to use unconventional commits these days.

@Lucianovici
Copy link
Contributor Author

Lucianovici commented Aug 12, 2021

Alright thank you, I will use the convention from now on. But why didn't it work, I just alter the history and force push it.

@myii
Copy link
Member

myii commented Aug 23, 2021

Just noting that this is an example of a patch for this Salt regression as mentioned here:

We may want to revert the with context changes if that upstream regression is fixed in an appropriate way.

@saltstack-formulas-travis

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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