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

feat(run): deprecate parameters in run function that are in context #483

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

joelwurtz
Copy link
Member

Goal is each parameters that is available in context should not be duplicated in the run function (or function depending on it), should simplify calls in the futures

Copy link
Member

@pyrech pyrech left a comment

Choose a reason for hiding this comment

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

I wonder whether the documentation should stay in the run section? If not, paragraph will need to be reworded to precise that it concerns the process execution through the run() helper.

LGTM otherwise

Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

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

LGTM too 👍🏼 same question as @pyrech

@joelwurtz joelwurtz force-pushed the feat/deprecate-run-parameter branch from 8ad1320 to 9dda382 Compare July 16, 2024 10:40
@joelwurtz
Copy link
Member Author

I was wondering the same about context, but since context may be used in other place that the run method, i think it's better to have documentation about it in the context section and not the run section, even if somes parameters are linked to the run function, they may be used when another method will use the run method (more it's helper but it's the same)

So for me documentation about those parameters should be in context section not in the run one, maybe we could add some links inside the run documentation to help this

@joelwurtz joelwurtz force-pushed the feat/deprecate-run-parameter branch from 9dda382 to 31f18e2 Compare August 1, 2024 09:12
Copy link
Member

@pyrech pyrech left a comment

Choose a reason for hiding this comment

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

LGTM

@lyrixx
Copy link
Member

lyrixx commented Aug 6, 2024

Could you rebase + add a note in the CHANGELOG?

@joelwurtz
Copy link
Member Author

done

@lyrixx lyrixx merged commit f180afd into main Aug 14, 2024
8 of 9 checks passed
@lyrixx lyrixx deleted the feat/deprecate-run-parameter branch August 14, 2024 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants