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.

Sign up for GitHub

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

Default value for scmInfo #117

Merged
merged 3 commits into from
Mar 28, 2017
Merged

Default value for scmInfo #117

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dac37c0
#133
Dec 16, 2016
f8a2a6b
Adding http[s] format support
Dec 23, 2016
c5e88fc
Added support for more url formats
Dec 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/main/scala/com/typesafe/sbt/SbtGit.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,18 @@ object SbtGit {
gitDescribedVersion := gitReader.value.withGit(_.describedVersion).map(v => git.gitTagToVersionNumber.value(v).getOrElse(v)),
gitCurrentTags := gitReader.value.withGit(_.currentTags),
gitCurrentBranch := Option(gitReader.value.withGit(_.branch)).getOrElse(""),
gitUncommittedChanges in ThisBuild := gitReader.value.withGit(_.hasUncommittedChanges)
gitUncommittedChanges in ThisBuild := gitReader.value.withGit(_.hasUncommittedChanges),
scmInfo := {
val remote = """origin[ \t]+git@([^:]*):(.*)\.git[ \t]+\(fetch\)""".r
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to also support at least http[s]:// and git:// remotes? SSH remotes can also start with ssh:// but I don't know if that is common.

Process("git remote -v").lines_!.collect {
Copy link
Member

Choose a reason for hiding this comment

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

If you just want the URL for origin git remote get-url origin will get you the first fetch URL.

Copy link
Author

@nscarcella nscarcella Dec 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Indeed, but it does not work for older versions of Git. I, for example, have 1.9.1 and the get-url subcommand is not available.

Copy link
Member

Choose a reason for hiding this comment

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

OK, then how about using git ls-remote --get-url origin added in git/git@45781ad and released in v1.7.5, which will also honor url.<base>.insteadOf.

Copy link
Author

Choose a reason for hiding this comment

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

Fair enough. I made a commit to change this and also refactored the code a bit, because the regex started to look quite ugly.

case remote(domain, repo) =>
ScmInfo(
url(s"https://$domain/$repo"),
s"scm:git:https://$domain/$repo.git",
Copy link
Member

Choose a reason for hiding this comment

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

Is the scm:git: prefix really needed? I thought it was only the Maven release plugin which used that convention.

I would also suggest to leave out the devConnection field and just use the URL returned either by git ls-remote --get-url origin or git remote -v as the connection URL. For GitHub (and I guess most other hosting services) SSH and HTTPS can both be used for read/write.

Finally, avoiding to hard-code the supported URLs would be nice, but I don't know what the goal of the plugin is in terms of what Git workflows should be supported.

Copy link
Author

Choose a reason for hiding this comment

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

Sadly it seems like this needs to be specified in this way to generate the pom properly. I couldn't make my Sonatype work otherwise.

Regarding the supported URLs, I do agree, though this change is only meant to offer a sane default. Any special cases can still be set manually.

Some(s"scm:git:git@$domain:$repo.git")
)
}.headOption
}
)
val projectSettings = Seq(
// Input task to run git commands directly.
Expand Down