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

Having "tag =" in subversion block may cause confusion #56

Open
mnlevy1981 opened this issue Jan 2, 2018 · 4 comments
Open

Having "tag =" in subversion block may cause confusion #56

mnlevy1981 opened this issue Jan 2, 2018 · 4 comments

Comments

@mnlevy1981
Copy link
Contributor

Summary of Issue:

Having tag = in a component block using protocol = svn is a little misleading; first of all, svn "tags" are really just branches combined with a web-hook to make them read-only. Also, it's not really clear where repo_url ends and tag begins. As far as I can tell, all of the following are the same

[pop]
tag = trunk_tags/cesm_pop_2_1_20171008
protocol = svn
repo_url = https://svn-ccsm-models.cgd.ucar.edu/pop2
local_path = components/pop
required = True
[pop]
tag = pop2/trunk_tags/cesm_pop_2_1_20171008
protocol = svn
repo_url = https://svn-ccsm-models.cgd.ucar.edu
local_path = components/pop
required = True
[pop]
tag = cesm_pop_2_1_20171008
protocol = svn
repo_url = https://svn-ccsm-models.cgd.ucar.edu/pop2/trunk_tags
local_path = components/pop
required = True

Expected behavior and actual behavior:

What I'd rather see is the full URL in repo_url and no tag field:

[pop]
protocol = svn
repo_url = https://svn-ccsm-models.cgd.ucar.edu/pop2/trunk_tags/cesm_pop_2_1_20171008
local_path = components/pop
required = True
@mnlevy1981
Copy link
Contributor Author

On a more nitpicky note, I think the following field ordering is more readable:

[pop]
local_path = components/pop
repo_url = https://svn-ccsm-models.cgd.ucar.edu/pop2/trunk_tags/cesm_pop_2_1_20171008
protocol = svn
required = True

From top to bottom, that's

  1. where is this external going?
  2. where is it coming from?
  3. How should I get it?
  4. Is it required?

For git-based externals, tag = (which could also be a branch, right? so maybe branch_or_tag =) is part of "where is it coming from" and should be listed between repo_url and protocol. For components with additional externals to manage, I think the best place for the optional external = is following protocol.

@billsacks
Copy link
Member

@mnlevy1981 I see your point about the equivalence of different paths for svn repo_url / tag. However, I feel that it should be kept the way it is for consistency between svn and git listings.

To reply to some of your specific points:

For git-based externals, tag = (which could also be a branch, right? so maybe branch_or_tag =)

For git, the semantics of branches and tags differ. e.g., see #34 (and also possibly #26 for some other subtleties).

Regarding field ordering: I like your idea of listing local_path at the top. To me it makes sense to have protocol listed before repo_url, though. I believe that manage_externals doesn't enforce any ordering, so this would just involve changes in the cesm repository.

@mnlevy1981
Copy link
Contributor Author

I see your point about the equivalence of different paths for svn repo_url / tag. However, I feel that it should be kept the way it is for consistency between svn and git listings.

If I'm reading #34 correctly, work is being done to support either

[$COMPNAME]
tag = $TAGNAME
protocol = git
repo_url = $REPO_URL
local_path = $LOCAL_PATH
required = True

or

[$COMPNAME]
branch = $BRANCHNAME
protocol = git
repo_url = $REPO_URL
local_path = $LOCAL_PATH
required = True

If that's an accurate statement, then it seems like you're introducing more functionality that is vital for git but not necessary for svn... and I see that as a stronger reason to omit fields from svn repositories. What if you named the fields git_tag and git_branch so it's clear that it's git-only?

@billsacks
Copy link
Member

@mnlevy1981 okay I see your point. I'd be okay with having this labeled git_tag or git_branch, but will defer to @bandre-ucar to weigh in on how hard this would be to implement.

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

No branches or pull requests

2 participants