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.

Sign up for GitHub

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

Jump to bottom

feat: experiment taskfile envs take precedence over os envs #1633

Merged
merged 4 commits into from
Jul 16, 2024
Merged

feat: experiment taskfile envs take precedence over os envs #1633

merged 4 commits into from
Jul 16, 2024

Conversation

vmaerten
Copy link
Member

@vmaerten vmaerten commented May 5, 2024 •
edited
Loading

Linked to :

I am totally open to rework this, including add new template function (as discussed in discord)

@vmaerten
Copy link
Member Author

vmaerten commented May 8, 2024

I managed to understand why, it's because the Init() from experiment is called before my SetEnv. I managed to make this test pass by instantiating an experiment in the test.

It's ready to review

@mgbowman
Copy link

mgbowman commented Jul 3, 2024

@andreynering @pd93 sorry to nag but my org could really use this functionality right now. Any chance one of you guys could review / comment on this?

andreynering
andreynering approved these changes Jul 3, 2024
View reviewed changes
Copy link
Member

@andreynering andreynering left a comment

Choose a reason for hiding this comment

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

Looks good to me.

This has conflicts to be fixed, and I'll also ask a review from @pd93 to get an extra opinion.

website/docs/experiments/env_precedence.mdx Outdated Show resolved Hide resolved
@andreynering andreynering requested a review from pd93 July 3, 2024 23:57
@andreynering andreynering mentioned this pull request Jul 3, 2024
Closed
3 tasks
vmaerten and others added 2 commits July 5, 2024 08:30
@vmaerten
Copy link
Member Author

vmaerten commented Jul 5, 2024

Thanks for the typo @andreynering
I've rebased and fixed conflict
I've also added a small paragraph about default

pd93
pd93 approved these changes Jul 16, 2024
View reviewed changes
Copy link
Member

@pd93 pd93 left a comment

Choose a reason for hiding this comment

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

No further comments from me for now. Let's get some feedback from some users and we can make decisions from there.

@pd93 pd93 enabled auto-merge (squash) July 16, 2024 22:43
@pd93 pd93 disabled auto-merge July 16, 2024 22:44
@pd93 pd93 merged commit 4b6c79a into go-task:main Jul 16, 2024
11 of 13 checks passed
pd93 added a commit that referenced this pull request Jul 16, 2024
@pd93
chore: changelog for #1633
1f2cbfb
@mgbowman
Copy link

Awesome 🚀

When can we expect an official release containing this new experiment?

@vmaerten
Copy link
Member Author

Thanks for your interest in this feature.

We cannot provide ETA (you can find more information in our doc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andreynering andreynering andreynering approved these changes

@pd93 pd93 pd93 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@vmaerten @mgbowman @andreynering @pd93