Fixed fetch issue #103

ThomasJejkal · 2018-01-16T06:41:27Z

Fixed bagDataDir argument in PayloadWriterTest#testWritePayloadFilesMinusFetchFiles.
Disabled removal of BagitWarning.DIFFERENT_NORMALIZATION in BagLinterTest as it seems to be produced also under MacOS in the meantime (resolves #102)
Fixed PayloadWriter#getFetchPaths to resolve fetch file path against the bag data folder correctly (resolves #99)

…inusFetchFiles. Disabled removal of BagitWarning.DIFFERENT_NORMALIZATION in BagLinterTest as it seems to be produced also under MacOS in the meantime (resolves #102) Fixed PayloadWriter#getFetchPaths to resolve fetch file path against the bag data folder correctly (resolves #99)

coveralls · 2018-01-16T10:53:50Z

Coverage remained the same at 96.947% when pulling 4d0834d on ThomasJejkal:fetch_fix into c863f0d on LibraryOfCongress:master.

ThomasJejkal · 2018-01-16T12:12:35Z

Obviously, the MacOS build in travis-ci uses a different OS version where removing BagitWarning.DIFFERENT_NORMALIZATION (see #102) is still necessary.

… to allow travis build Added MessageBundle_de_DE.properties for German localization Fixed minor typos in MessageBundle.properties

johnscancella · 2018-01-17T12:39:00Z

src/main/resources/MessageBundle.properties

@@ -200,7 +200,7 @@ writing_line_to_file=Writing line [{}] to [{}]

 #for BagWriter.java
 writing_payload_files=Writing payload files.
-upsert_payload_oxum=Upserting payload-oxum.
+upsert_payload_oxum=Inserting payload-oxum.


No, upserting is correct. It stands for inserting or updating.

Sorry and thanks for extending my English vocabulary. 👍

👍 . And thank you for catching all my english mistakes!

Who are the intended audience for this message? If not, I'd be inclined to use something like “updating” since “upsert” is generally only familiar to people who work with databases a lot.

It is a DEBUG message, which I generally wrote for other programmers.

coveralls · 2018-01-17T12:42:40Z

Coverage remained the same at 96.947% when pulling f4ec574 on ThomasJejkal:fetch_fix into c863f0d on LibraryOfCongress:master.

coveralls · 2018-01-17T12:42:41Z

Coverage remained the same at 96.947% when pulling f4ec574 on ThomasJejkal:fetch_fix into c863f0d on LibraryOfCongress:master.

johnscancella

It seems that you changed the spacing for tabs to be 4 spaces among other changes, please return the formatting to what it originally was. Thanks!

Fixed wrong tab size for indention

johnscancella · 2018-01-17T13:20:22Z

Is there a reason you keep changing the spacing and other formatting changes?

ThomasJejkal · 2018-01-17T13:27:27Z

Actually, I'm using the auto-indent feature of Netbeans and by default, Netbeans is using its formatting rules from the official Java Code Conventions saying e.g. that a tab has a size of 4 spaces. I've changed the tab size to 3 but obviously you are using 2, so I'll change, reformat and commit again.

coveralls · 2018-01-17T13:34:57Z

Coverage remained the same at 96.947% when pulling 9695dad on ThomasJejkal:fetch_fix into c863f0d on LibraryOfCongress:master.

coveralls · 2018-01-17T13:38:24Z

Coverage remained the same at 96.947% when pulling 0761353 on ThomasJejkal:fetch_fix into c863f0d on LibraryOfCongress:master.

johnscancella · 2018-01-17T13:39:31Z

Ah ok, I am using eclipse. If I recall correctly you can import eclipse formatting? If so you are welcome to use my formatter (just rename to .xml): eclipse formater.xml.txt

It just makes it hard to review your actual changes since there is so much superfluous highlighting from the format changes.

ThomasJejkal · 2018-01-17T14:00:45Z

I've now committed again using your configuration. I hope this helped.

johnscancella · 2018-01-17T19:07:23Z

src/main/java/gov/loc/repository/bagit/conformance/ManifestChecker.java

@@ -20,134 +20,134 @@
 import gov.loc.repository.bagit.util.PathUtils;

 /**
- * Part of the BagIt conformance suite. 
- * This checker checks for various problems related to the manifests in a bag.


as far as I can tell, this file only contains formatting changes, is that correct?

johnscancella · 2018-01-17T19:08:07Z

src/main/java/gov/loc/repository/bagit/conformance/MetadataChecker.java

@@ -16,38 +16,39 @@
 import gov.loc.repository.bagit.reader.MetadataReader;

 /**
- * Part of the BagIt conformance suite. 


As far as I can tell, this file only contains formatting changes, is that correct?

johnscancella · 2018-01-17T19:08:58Z

src/main/java/gov/loc/repository/bagit/writer/PayloadWriter.java

@@ -21,73 +21,80 @@
 * Responsible for writing out the bag payload to the filesystem
 */
 public final class PayloadWriter {
+


As far as I can tell, this file only contains formatting changes, is that correct?

johnscancella · 2018-01-17T19:09:57Z

src/test/java/gov/loc/repository/bagit/conformance/BagLinterTest.java

@@ -3,7 +3,6 @@
 import java.io.File;


I believe this file only contains formatting changes, is that correct?

johnscancella · 2018-01-17T19:14:19Z

Hi @ThomasJejkal it looks like some of the formatting that changed is still there. Could you take another pass at it so only your logical changes show up? That way it will make it much easier for me to review. Thanks!

coveralls · 2018-01-17T20:45:03Z

Coverage remained the same at 96.947% when pulling 7b2ce33 on ThomasJejkal:fetch_fix into c863f0d on LibraryOfCongress:master.

ThomasJejkal · 2018-01-18T06:20:47Z

I've created a new pull request with a clean copy as fixing all the small inconsistencies between both formatting schemes, even when using the same configuration, seems too frustrating to me.

Removed DIFFERENT_NORMALIZATION from expected warnings again in order…

f4ec574

… to allow travis build Added MessageBundle_de_DE.properties for German localization Fixed minor typos in MessageBundle.properties

johnscancella reviewed Jan 17, 2018

View reviewed changes

johnscancella suggested changes Jan 17, 2018

View reviewed changes

Replaced Inserting with Upserting again in MessageBundle.properties

9695dad

Fixed wrong tab size for indention

Fixed tab size for indention to 2

0761353

Applied Eclipse formatting

7b2ce33

johnscancella reviewed Jan 17, 2018

View reviewed changes

ThomasJejkal closed this Jan 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed fetch issue #103

Fixed fetch issue #103

ThomasJejkal commented Jan 16, 2018

coveralls commented Jan 16, 2018 •

edited

Loading

ThomasJejkal commented Jan 16, 2018

johnscancella Jan 17, 2018

ThomasJejkal Jan 17, 2018

johnscancella Jan 17, 2018 •

edited

Loading

acdha Jan 17, 2018

johnscancella Jan 17, 2018

coveralls commented Jan 17, 2018 •

edited

Loading

coveralls commented Jan 17, 2018

johnscancella left a comment

johnscancella commented Jan 17, 2018

ThomasJejkal commented Jan 17, 2018

coveralls commented Jan 17, 2018 •

edited

Loading

coveralls commented Jan 17, 2018 •

edited

Loading

johnscancella commented Jan 17, 2018

ThomasJejkal commented Jan 17, 2018

johnscancella Jan 17, 2018

johnscancella Jan 17, 2018

johnscancella Jan 17, 2018

johnscancella Jan 17, 2018

johnscancella commented Jan 17, 2018

coveralls commented Jan 17, 2018 •

edited

Loading

ThomasJejkal commented Jan 18, 2018

Fixed fetch issue #103

Fixed fetch issue #103

Conversation

ThomasJejkal commented Jan 16, 2018

coveralls commented Jan 16, 2018 • edited Loading

ThomasJejkal commented Jan 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnscancella Jan 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jan 17, 2018 • edited Loading

coveralls commented Jan 17, 2018

johnscancella left a comment

Choose a reason for hiding this comment

johnscancella commented Jan 17, 2018

ThomasJejkal commented Jan 17, 2018

coveralls commented Jan 17, 2018 • edited Loading

coveralls commented Jan 17, 2018 • edited Loading

johnscancella commented Jan 17, 2018

ThomasJejkal commented Jan 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnscancella commented Jan 17, 2018

coveralls commented Jan 17, 2018 • edited Loading

ThomasJejkal commented Jan 18, 2018

coveralls commented Jan 16, 2018 •

edited

Loading

johnscancella Jan 17, 2018 •

edited

Loading

coveralls commented Jan 17, 2018 •

edited

Loading

coveralls commented Jan 17, 2018 •

edited

Loading

coveralls commented Jan 17, 2018 •

edited

Loading

coveralls commented Jan 17, 2018 •

edited

Loading