search

OCFL / ocfl-java

A Java OCFL implementation
MIT License
18 stars 12 forks source link

NTuple Omit Prefix Implementation #58

Closed ives1227 closed 2 years ago

ives1227 commented 2 years ago

This is the implementation of the recently merged spec https://github.com/OCFL/extensions/blob/main/docs/0007-n-tuple-omit-prefix-storage-layout.md.

ives1227 commented 2 years ago

@pwinckles I've addressed all of the above comments and recommitted. Thanks so much for your timely review!

ives1227 commented 2 years ago

@pwinckles I'll get to these later tonight or tomorrow. Thanks!

pwinckles commented 2 years ago

I just committed some checkstyle rules that you can pickup if you rebase. I'm not a dogmatic code style person, but it is nice if fundamental things like spacing is consistent.

ives1227 commented 2 years ago

@pwinckles I made the requested changes again. Thanks for the checkstyles - that helped me clear up the discrepancies.

pwinckles commented 2 years ago

It appears that your PR is now including commits that were made to main. You need to rebase your PR so that it only includes your commits.

ives1227 commented 2 years ago

@pwinckles I was just typing this to you. I rebased on main because of the checkstyles and didn't realize it included all of these changes. I'll fix this when I have a chance.

pwinckles commented 2 years ago

@ives1227 I'm hoping to get a release out this week or next. Do you think you'll have time to rebase this PR?

ives1227 commented 2 years ago

@pwinckles I will try to get to this in the next day or two. Between the holidays and work demands, I didn't have a chance to look at this.

ives1227 commented 2 years ago

@pwinckles I believe this is all set now.

pwinckles commented 2 years ago

You seem to have reverted the PR back to its original state, undoing all of the revisions that were made as part of the review process

pwinckles commented 2 years ago

I created an example branch of what this should look like. Note, that the tabs are still not fixed in that branch.

I created that checking out your branch and doing the following:

# Rebase to remove the three most recent commits
git rebase -i HEAD~4

# Add the upstream if you don't have it
git remote add upstream git@github.com:UW-Madison-Library/ocfl-java.git

# Rebase from upstream
git rebase upstream/main

That should produce the same branch as mine. Then, when you push to origin to update this PR you'll need to use the --force flag.

ives1227 commented 2 years ago

@pwinckles the files I had previously were 1.4.2-SNAPSHOT but when I rebased from your branch, they were 1.4.3-SNAPSHOT. So, I should be using 1.4.3-SNAPSHOT instead?

pwinckles commented 2 years ago

@pwinckles the files I had previously were 1.4.2-SNAPSHOT but when I rebased from your branch, they were 1.4.3-SNAPSHOT. So, I should be using 1.4.3-SNAPSHOT instead?

Yes, 1.4.3-SNAPSHOT is correct

ives1227 commented 2 years ago

@pwinckles I put the correct snapshot version back in.

ives1227 commented 2 years ago

@pwinckles thank you!