search

saltstack-formulas / node-formula

Manage node.js with SaltStack
http://docs.saltstack.com/en/latest/topics/development/conventions/formulas.html
Other
26 stars 104 forks source link

Import YAML with context #56

Closed Lucianovici closed 3 years ago

Lucianovici commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

@noelmcloughlin Since I am a casual committer, can we do as advice here please? https://www.conventionalcommits.org/en/v1.0.0/#do-all-my-contributors-need-to-use-the-conventional-commits-specification

noelmcloughlin commented 3 years ago

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. https://github.com/saltstack-formulas/node-formula/blob/79081c913440a83104ed3b7347c777e2eb9f4773/CODEOWNERS#L11-L12

noelmcloughlin commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

:tada: This PR is included in version 1.1.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: