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

fix-extraEnv-default-value #329

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

ruckc
Copy link
Contributor

@ruckc ruckc commented Apr 19, 2022

Summary

I messed up the default value for extraEnv in #316. This fixes the jobserver.extraEnv to be an array, which is what the template is expecting

Without this change, if trying to use jobserver's extraEnvs, you get this error:

cannot overwrite table with non table for mattermost-enterprise-edition.global.features.jobserver.extraEnv

I messed up the default value for extraEnv. This fixes the jobserver.extraEnv to be an array, which is what the template is expecting.
@mattermod
Copy link
Contributor

Hello @ruckc,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@ruckc
Copy link
Contributor Author

ruckc commented Apr 20, 2022

the tests need to be retried

@ruckc
Copy link
Contributor Author

ruckc commented Apr 29, 2022

@spirosoik / @pfltdv / @stylianosrigas - can you review?

@ghost ghost requested review from phoinixgrr, spirosoik, stylianosrigas and a user May 6, 2022 07:52
@spirosoik
Copy link
Member

@pfltdv did you give a try to test locally?

@ghost
Copy link

ghost commented May 10, 2022

@pfltdv did you give a try to test locally?

@spirosoik Yes tested with and without jobserver extra vars. Had no issue at both executions.

Copy link
Contributor

@phoinixgrr phoinixgrr left a comment

Choose a reason for hiding this comment

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

@ruckc

Fine with merging this.

Would you mind also commenting @ values.yaml some examples on how the Jobserver extraEnv directive can be used?
For Example:
https://github.com/mattermost/mattermost-helm/blob/master/charts/mattermost-enterprise-edition/values.yaml#L245-L251

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@jonathanwiemers
Copy link
Contributor

@ruckc any reason this wasn't merged / updated anymore? Just saw that i've just create a PR for the same issue.

@ruckc
Copy link
Contributor Author

ruckc commented Jul 11, 2022

@jonathanwiemers - no real reason, other than the 3 week lag on feedback. My need for this was removed as we moved away from mattermost due to the integration and license headaches.

@spirosoik
Copy link
Member

/update-branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants