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

Add feature: sbt upgradeBuildFile #68

Closed
wants to merge 2 commits into from

Conversation

shigemk2
Copy link
Contributor

I add a feature to upgrade build file.

sbt-update does not have a feature to upgrade build file just like npm-check-updates:
https://github.com/tjunnone/npm-check-updates

npm-check-updates has a feature to upgrade package.json when some packages have update.
So I wanted a feature to upgrade build file.

We can upgrade build file by sbt upgradeBuildFile.

Default build file is build.sbt, but there are many projects whose build file is not build.sbt. So I let them to set build file location in dependencyUpdatesReportFile.

@rtimush
Copy link
Owner

rtimush commented Jan 24, 2017

Thank you for the pull request! I will take a look as soon as I have some time.

@shigemk2
Copy link
Contributor Author

Thank you for your comment. Please review my PR!

@rtimush
Copy link
Owner

rtimush commented Feb 18, 2017

Thank you again for your contribution. While I think that upgradeBuildFile may be useful, there are some concerns:

  1. Dependency versions are often declared as variables, upgradeBuildFile will not work in such cases. This is understandable but limits the command usability.
  2. Build file update process based on string match and replace is very fragile. Sometimes it will fail to update a dependency (say, if there is a line break), sometimes it will result in incorrect file updates, which is much worse.

Given that, I think that upgradeBuildFile in this implementation will result in constant uncertainty whether it will work this time or not, so I tend to refrain from merging this pull request.

@jozic
Copy link
Contributor

jozic commented Feb 18, 2017

@shigemk2
probably bunch of (passing) scripted tests can address concern 2

@rtimush rtimush closed this Aug 10, 2017
@shigemk2
Copy link
Contributor Author

I am very sorry. I forgot this PR's comment.
I am not sure whether I resolve two concerns you showed now.
So, this PR is incomplete.

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

Successfully merging this pull request may close these issues.

None yet

3 participants