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

Clean up name delegation in the plugin. A few rules: #250

Merged
merged 7 commits into from
Jul 19, 2014

Conversation

jsuereth
Copy link
Member

  • Each configuration should have its own name and normalizedName instance
  • Debian/Rpm delegate to Linux which delegates to project name.
  • Universal has its own name, which is used for generating BASH/BAT files
  • Separate key for the package filename now
  • Windows has its own name/normalized name which delegates to raw.

Fixes #188

* Each configuration should have its own `name` and `normalizedName` instance
* Debian/Rpm delegate to Linux which delegates to project name.
* Universal has its own name, which is used for generating BASH/BAT files
* Separate key for the package filename now
* Windows has its own name/normalized name which delegates to raw.

Fixes #188
@jsuereth
Copy link
Member Author

Review by @muuki88 cc> @timperrett

packageSummary in Rpm <<= packageSummary in Linux,
packageDescription in Rpm <<= packageDescription in Linux,
target in Rpm <<= target(_ / "rpm")
target in Rpm <<= target(_ / "rpm"),
name in Rpm <<= name in Linux
Copy link
Contributor

Choose a reason for hiding this comment

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

so if I specify name in RPM := "xyz" it will still work?

Copy link
Member Author

Choose a reason for hiding this comment

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

it should. It will get normalized, which is something we may want to rethink soon.

@muuki88
Copy link
Contributor

muuki88 commented May 16, 2014

Hmmm. I'm thinking about introducing a new Setting packageName. By default this is the normalizedName, but you can override it. So setting packageName would by pass all name and normalizedName tasks, which IMHO seem to cause all this troubles.

Then we can just use packageName everywhere without a scope, right? If I run debian:* packageName will automatically be resolved in the debian scope.

@kardapoltsev
Copy link
Member

Excuse me that was a stupid mistake on buttons).

@jsuereth
Copy link
Member Author

@muuki88 Yeah, I think that's a far better idea.

What we should do perhaps:

name in <Scope> - A name used when creating the file for a package
packageName - The name we use for the script/directory/etc. of an archetype (like the JavaArchetype.

Sound reasonable?

@kardapoltsev
Copy link
Member

Sure!)

@timperrett
Copy link

+1 for packageName, that would be vastly simpler

Conflicts:
	src/main/scala/com/typesafe/sbt/packager/archetypes/JavaServerApplication.scala
	src/main/scala/com/typesafe/sbt/packager/rpm/RpmPlugin.scala
	src/sbt-test/debian/test-mapping/test
@muuki88
Copy link
Contributor

muuki88 commented Jun 30, 2014

I merged the current master into this pr. What has to be done here?

@fiadliel
Copy link
Contributor

fiadliel commented Jul 2, 2014

Project.normalizeModuleID only exists in SBT 0.13. Possibly use StringUtilities.normalize instead (it's deprecated, but present in SBT 0.12)?

@muuki88
Copy link
Contributor

muuki88 commented Jul 2, 2014

What are you referring to?

I went through the discussion and will start to implement the packageName setting soon.

muuki88 added 2 commits July 14, 2014 23:06
Conflicts:
	src/main/scala/com/typesafe/sbt/packager/debian/DebianPlugin.scala
@pcting
Copy link
Contributor

pcting commented Jul 17, 2014

@jsuereth, so this is my use case: #296

What are your thoughts on adding additional keys such as logPath and applicationPath and have their values defaulted to packageName? This will allow the log directory and application install directory to be different.

What I don't like about this is that there's already a defaultLinuxLogsLocation and a defaultLinuxInstallLocation, which may cause a lot of user confusion.

@muuki88
Copy link
Contributor

muuki88 commented Jul 18, 2014

@pcting , once we detangled everything from the normalizedName key, it should be easier to add configuration settings like this. Thanks for your feedback :)

@jsuereth
Copy link
Member Author

@muuki88 your changes LGTM!!

@muuki88
Copy link
Contributor

muuki88 commented Jul 18, 2014

So, this should be it. I'm currently working on a documentation, which will also be the starting point for the next configuration refactorings. @jsuereth , @aparkinson , @kardapoltsev I added a lot of tests, but nothing guaranteed .

muuki88 added a commit that referenced this pull request Jul 18, 2014
@muuki88 muuki88 added the v0.7.x label Jul 19, 2014
@muuki88
Copy link
Contributor

muuki88 commented Jul 19, 2014

One last thing @jsuereth . Is this binary compatible? Can it be merged in the 0.7.x branch?

@jsuereth
Copy link
Member Author

@muuki88 Yeah, as long as it's just new keys and modifying existing Setting[_] objects, it's BC. Method changes, or removal of keys is how you break BC. The sbt Setting[] + Key[] setup leads us a bit more resiliency to bin-compat, but glad you asked.

jsuereth added a commit that referenced this pull request Jul 19, 2014
Clean up name delegation in the plugin.  A few rules:
@jsuereth jsuereth merged commit 53d3cd8 into master Jul 19, 2014
@jsuereth jsuereth deleted the wip/name-delegation branch July 19, 2014 12:09
jsuereth added a commit that referenced this pull request Jul 19, 2014
muuki88 referenced this pull request Jul 20, 2014
@muuki88 muuki88 removed the v0.7.x label Jul 21, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"name in Linux := " ignored
6 participants