-
Notifications
You must be signed in to change notification settings - Fork 597
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
Incorporate lessons learned from latest publishing. #693
Conversation
…e some settings to common.
…o 3.0-BETA-SNAPSHOT so we can update "in-place".
being lost in Driver.execute before firrtl is called.
# Conflicts: # build.sbt
build.sbt
Outdated
|
||
// Provide a managed dependency on X if -DXVersion="" is supplied on the command line. | ||
libraryDependencies ++= (Seq("firrtl").map { | ||
dep: String => "edu.berkeley.cs" %% dep % sys.props.getOrElse(dep + "Version", defaultVersions(dep)) }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this line be tabbed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, this should be tabbed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly looks reasonable, two style comments.
build.sbt
Outdated
|
||
// Provide a managed dependency on X if -DXVersion="" is supplied on the command line. | ||
libraryDependencies ++= (Seq("firrtl").map { | ||
dep: String => "edu.berkeley.cs" %% dep % sys.props.getOrElse(dep + "Version", defaultVersions(dep)) }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, this should be tabbed
build.sbt
Outdated
|
||
// Provide a managed dependency on X if -DXVersion="" is supplied on the command line. | ||
libraryDependencies ++= (Seq("firrtl").map { | ||
dep: String => "edu.berkeley.cs" %% dep % sys.props.getOrElse(dep + "Version", defaultVersions(dep)) }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also merge the dep into the rest of them? Unless you anticipate dependencies on more than just firrtl, that map really isn't doing much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right. It was mostly to preserve style with the other repos where the map includes more/all of the BIG4.
# Conflicts: # build.sbt
# Conflicts: # build.sbt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not completely sure what this does but it looks sane.
Changes to sbt incorporated from release branch. This is not strictly required, but is an attempt to keep master and release branches relatively in sync.