[GitHub] [commons-imaging] kinow commented on issue #49: Add support for EXIF 2.32
kinow commented on issue #49: Add support for EXIF 2.32 URL: https://github.com/apache/commons-imaging/pull/49#issuecomment-535682893 Hi @GITNE >I have added terminal newlines where applicable but Travis CI still fails for some unrelated reason. You have to rebase your branch on master. I've fixed the Javadoc error on JVM 13. So once rebased, this PR should pass Travis with no issues 🤞 . Do you intend to modify anything else in the PR? Add any feature/code? >Btw, personally I consider tools which require you to put a terminal newline in your files to be broken or to have a badly implemented parser. That pseudo argument for terminal newlines where stream processing concatenated files my lead to undesired behavior is dumb. It is naive to assume that processing concatenated files and processing files separately will or should always produce identical results. It is the file format that governs parsing not the mode of processing. Just to clarify, the newline here is for git. I believe it came from unix where it was good practice to have the newline. Not sure if there's a practical reason for that. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [commons-imaging] kinow commented on issue #49: Add support for EXIF 2.32
kinow commented on issue #49: Add support for EXIF 2.32 URL: https://github.com/apache/commons-imaging/pull/49#issuecomment-523195306 @GITNE it's a checkstyle issue. If you click on "Details" next to the Travis CI check (👇 ) there should be a few jobs within this PR's build. Looking at a random job's log, I see: ``` [ERROR] src/main/java/org/apache/commons/imaging/formats/tiff/constants/ExifTagConstants.java:[0] (misc) NewlineAtEndOfFile: File does not end with a newline. ``` Looking at the changed files in this PR, it's possible to spot that your editor probably removed the [empty line at the end of the file](https://unix.stackexchange.com/a/18789/33951). If you add that back, squash your commit, and push, Travis should kick again and hopefully pass this time 🤞 BTW, I've been holding back reviewing this one until you confirmed you got everything ready. I remember you mentioning you wanted to add something more... so just let us know once it's ready for review :) Cheers Bruno This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [commons-imaging] kinow commented on issue #49: Add support for EXIF 2.32
kinow commented on issue #49: Add support for EXIF 2.32 URL: https://github.com/apache/commons-imaging/pull/49#issuecomment-510173518 Yeah, that's a not very nice issue finsing the tags... but I think that's probably better done in another PR later. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [commons-imaging] kinow commented on issue #49: Add support for EXIF 2.32
kinow commented on issue #49: Add support for EXIF 2.32 URL: https://github.com/apache/commons-imaging/pull/49#issuecomment-509885960 One test failing on Travis: ``` [ERROR] Failures: [ERROR] TiffTagIntegrityTest.testTagIntegrity:62->verifyFields:115 Missing tag 34867 [INFO] [ERROR] Tests run: 615, Failures: 1, Errors: 0, Skipped: 16 [INFO] [INFO] [INFO] BUILD FAILURE [INFO] ``` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [commons-imaging] kinow commented on issue #49: Add support for EXIF 2.32
kinow commented on issue #49: Add support for EXIF 2.32 URL: https://github.com/apache/commons-imaging/pull/49#issuecomment-506285856 @GITNE I think we are good with breaking changes before the final 1.0 release (see mailing archive: https://markmail.org/thread/dszscnnjpy5shu5f). Feel free to complete the PR with any other changes you may find useful. Just let us know once it's ready for review 👍 And thanks for your contribution! Bruno This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [commons-imaging] kinow commented on issue #49: Add support for EXIF 2.32
kinow commented on issue #49: Add support for EXIF 2.32 URL: https://github.com/apache/commons-imaging/pull/49#issuecomment-506011709 Hi @GITNE Thanks a lot for your contribution. I am currently overseas, but planning to look into this PR as soon as I have some spare time. On backward compatibility, thanks a lot for taking that into consideration. That's an important point. We have added 1 backward **incompatible** change after the alpha1 release. Which means that the alpha2 release would not be backward compatible. There was some discussion in the mailing list on whether it would be OK or not to add such changes in alpha releases. I believe for now we are going with a per-component policy, where alpha releases in Commons Imaging would not be behaviour/binary backward compatible (we still have time to discuss that before alpha2). I'm writing to that thread about this PR, to confirm whether we are keeping BC or not. If not, it would be great if you would be willing to complete the work for fully compliance with the standard. Thanks Bruno - https://markmail.org/thread/drivqucdadxzfb3i This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
