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

multiple cml pr runs #794

Open
DavidGOrtega opened this issue Nov 1, 2021 · 6 comments
Open

multiple cml pr runs #794

DavidGOrtega opened this issue Nov 1, 2021 · 6 comments
Assignees
Labels
bug Something isn't working cml-pr Subcommand p1-important High priority

Comments

@DavidGOrtega
Copy link
Contributor

If the data is changed after a PR it's going to run again generating strange branch names.

experiment-cml-pr-da72cf16-cml-pr-da72cf16-cml-pr-da72cf16

Things to consider:

  • If we revert the state use send-comment to comment into the new PR via cml send-comment --pr --commit-sha HEAD wont work
  • Do not take into account current branch name to avoid concatenation
  • Take into account file changes to generate the branch name also we have to consider cml pr: Allow users to configure P.R. title / description #792

I wonder if is practical to fix it, I mean, should cml pr allow double execution? Which ones would be those plausible scenarios?

@DavidGOrtega DavidGOrtega added cml-pr Subcommand bug Something isn't working p1-important High priority labels Nov 1, 2021
@0x2b3bfa0
Copy link
Member

Related to / duplicate of #758?

@DavidGOrtega
Copy link
Contributor Author

DavidGOrtega commented Nov 1, 2021

Related (not duplicated) to

@0x2b3bfa0
Copy link
Member

Enough material for an epic? 🤔

@DavidGOrtega
Copy link
Contributor Author

@0x2b3bfa0 Im attacking them in a PR

@casperdcl
Copy link
Contributor

@DavidGOrtega since there are lots of approaches I'd suggest writing CLI docstrings for your ideas so we can discuss them before spending time implementing a polished PR.

@DavidGOrtega
Copy link
Contributor Author

DavidGOrtega commented Nov 5, 2021

@casperdcl not really that complex, assuming that we are not rolling back the only issue here is more:

  • a naming issue
  • the only rollback to have into account is the user.name user.email
  • add the parameters to even allow the pr name as @daavoo suggested.

I would not consider it even an epic in terms of big.

@DavidGOrtega DavidGOrtega added cml-comment Subcommand and removed cml-comment Subcommand labels Nov 10, 2021
@casperdcl casperdcl changed the title cml pr does not properly work with multiple runs multiple cml pr runs Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cml-pr Subcommand p1-important High priority
Projects
None yet
Development

No branches or pull requests

3 participants