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

New secret rudder #662

Merged
merged 2 commits into from
Mar 30, 2023
Merged

New secret rudder #662

merged 2 commits into from
Mar 30, 2023

Conversation

trilopin
Copy link
Contributor

@trilopin trilopin commented Mar 28, 2023

Summary

After some name changing, MM_RUDDER_WRITE_KEY has become MM_RUDDER_PLUGINS_PROD.

Fix custom.mk so we get the right data.

Ticket Link

no

@trilopin trilopin requested a review from hanzei as a code owner March 28, 2023 16:39
@trilopin trilopin added this to the v2.1.5 milestone Mar 28, 2023
@trilopin trilopin added the 2: Dev Review Requires review by a core committer label Mar 28, 2023
@hanzei
Copy link
Contributor

hanzei commented Mar 28, 2023

@mickmister
Copy link
Member

FYI: This is another way to fix the issue: https://github.com/mattermost/mattermost-plugin-apps/blob/da73e99d66c1fd8bfad56282c55b145c8fa3076d/.github/workflows/cd.yaml#L18

@hanzei I think we should be consistent across the projects, and so I think this PR should implement this in the workflow file as you linked, instead of editing custom.mk. @trilopin Thoughts on this?

@trilopin
Copy link
Contributor Author

@hanzei I think we should be consistent across the projects, and so I think this PR should implement this in the workflow file as you linked, instead of editing custom.mk. @trilopin Thoughts on this?

💯 to have same solution to all repos.

I don't like to change custom.mk, but wouldn't we be making one more indirection (aka magic) by wiring MM_RUDDER_WRITE_KEY: ${{ secrets. MM_RUDDER_PLUGINS_PROD }} ? I'd understand this if we could do it just once for all plugins but it seems that we have to change all cd.yml anyway.

Is there any drawback on the custom.mk approach am I missing?

@trilopin trilopin merged commit 59e57d9 into master Mar 30, 2023
@trilopin trilopin deleted the new_secret_rudder branch March 30, 2023 09:14
@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Mar 30, 2023
mickmister pushed a commit that referenced this pull request Mar 30, 2023
After some name changing, MM_RUDDER_WRITE_KEY has become MM_RUDDER_PLUGINS_PROD.

Fix custom.mk so we get the right data.
@mickmister mickmister mentioned this pull request May 3, 2023
@avas27JTG avas27JTG mentioned this pull request Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants