Open mnlevy1981 opened 6 years ago
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
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
.
@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.
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?
@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.
Summary of Issue:
Having
tag =
in a component block usingprotocol = 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 whererepo_url
ends andtag
begins. As far as I can tell, all of the following are the sameExpected behavior and actual behavior:
What I'd rather see is the full URL in
repo_url
and notag
field: