Skip to content

Fix incompatibility to new scijava LogService#2

Merged
ctrueden merged 2 commits intoscijava:masterfrom
maarzt:log-service-revamp
Dec 12, 2017
Merged

Fix incompatibility to new scijava LogService#2
ctrueden merged 2 commits intoscijava:masterfrom
maarzt:log-service-revamp

Conversation

@maarzt
Copy link
Contributor

@maarzt maarzt commented Dec 5, 2017

Add implementations for the methods that were recently added to LogService.

A new version of scijva-log-slf4j containing this changes should be released together with scijava-common 2.66.2.

@ctrueden
Copy link
Member

ctrueden commented Dec 5, 2017

Thanks @maarzt. I am releasing scijava-common 2.67.0 now (there was new API added, so a minor version bump was necessary). Please rebase this branch to eliminate the WIP:

  • git fetch && git rebase -i origin/master.
  • Move the WIP to the first commit in the list and change it to e.
  • Save and quit to run the rebase.
  • Update the POM to have <scijava-common.version>2.67.0</scijava-common.version> in the <properties> section. Commit it, changing the message accordingly. Note that you should not hardcode the <version> of any dependency, because then the melting-pot and similar tooling can no longer override the versions used on the CLI.
  • git rebase --continue

It should then be the case that all commits compile with passing tests; the check-branch.sh script will verify this.

@maarzt
Copy link
Contributor Author

maarzt commented Dec 6, 2017

Thanks for releasing the new scijava-common version. I did the rebase as you described.
But not all commits compile:

  • The first commit "POM: depend on scijava-common version 2.67.0" breaks the code. (SLF4JLogService has no implementations for new LogService methods like LogService::subLogger ...)

  • The last commit "SLF4JLogService: support new LogService" fixes this.

Should I squash the two commits?

@ctrueden
Copy link
Member

ctrueden commented Dec 8, 2017

Should I squash the two commits?

Sure, that's usually what we do when code changes need to happen concurrently with a dependency version update. So squashing the first two commits together, and keeping the second commit message, sounds great. Thanks.

One small typo in the message: where -> were.

SLF4JLogService had no implementation for methodes that were added to
LogService in scijava-common version 2.66.1.

SLF4JLogService now implements all required methods because it extends
AbstractLogService.
@maarzt
Copy link
Contributor Author

maarzt commented Dec 11, 2017

I think this branch is ready to be merged

@ctrueden ctrueden merged commit 070bd89 into scijava:master Dec 12, 2017
@ctrueden
Copy link
Member

Thanks @maarzt!

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.

2 participants