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

brancher: potential collision with "workspace" tags/branches #3943

Closed
efiop opened this issue Jun 3, 2020 · 3 comments
Closed

brancher: potential collision with "workspace" tags/branches #3943

efiop opened this issue Jun 3, 2020 · 3 comments
Labels
bug Did we break something? p2-medium Medium priority, should be done, but less important

Comments

@efiop
Copy link
Member

efiop commented Jun 3, 2020

#3914 (comment) Kudos @Suor

@efiop efiop added bug Did we break something? p2-medium Medium priority, should be done, but less important triage Needs to be triaged labels Jun 3, 2020
@triage-new-issues triage-new-issues bot removed triage Needs to be triaged labels Jun 3, 2020
@Suor
Copy link
Contributor

Suor commented Jun 3, 2020

There are two subissues caused by this name clash:

Input one. When someone uses repo.brancher(revs=[..., "workspace"]) it's unclear whether he or she wants workspace to be included or there is a tag/branch named "workspace", which needs to be included.

Output one. When brancher yields "workspace" does it show workspace or a tag/branch with that name? This further confuses things like metrics/plots show/diff output and corresponding Repo.metrics.show()/Repo.params.show().

Also, we sometime refer to workspace as "" (empty string) for whatever reasons.

@efiop
Copy link
Member Author

efiop commented Jun 3, 2020

We could mark it with some char that is forbidden for tags. With working tree it was the space that was illegal, so we didn't have any issues. We could do something similar by including some rare illegal character somewhere or just using a flag when browsing and then on collision add something like workspace(working tree)(or tag/branch) or something.

@Suor
Copy link
Contributor

Suor commented Jun 5, 2020

If we don't care about presentation we can make some str subclass:

class Workspace(str):
    pass
WORKSPACE = Workspace("workspace")

Will work both for in and out. Looks as hacky as special char to me.

On the other hand the most clean solution I can think of is use flag for input and empty string for output. CLI should show workspace or something instead of empty string and it should go first.

@mattseddon mattseddon closed this as not planned Won't fix, can't repro, duplicate, stale Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Did we break something? p2-medium Medium priority, should be done, but less important
Projects
None yet
Development

No branches or pull requests

3 participants