search

sambitdash / PDFIO.jl

PDF Reader Library for Native Julia.
Other
128 stars 13 forks source link

Revert "Implemented == and < for CDDate" #34

Closed sambitdash closed 5 years ago

sambitdash commented 5 years ago

Reverts sambitdash/PDFIO.jl#32

gwierzchowski commented 5 years ago

@sambitdash Hi, Why it was all reverted ?

sambitdash commented 5 years ago

I was trying to merge the check-ins to one single check-in per merge but guess once you merge reverting creates a new entry and it was not much of use.

gwierzchowski commented 5 years ago

Thanks. So please let me know how to proceed now to reapply my changes. Should I re-base master in my repo from your current head, then create new branch and merge back changes with your changes done as code review and then create new PR? PS: Note that #30 should go first, as in unit test for #25 (not pushed yet) I need date comparison. BTW: I have also written on side new code which extracts TOC (Outline) from PDF, but this is for later.

sambitdash commented 5 years ago

Both #25 and #30 as far as your changes go are already in place. So merging back from sambitdash/PDFIO should not create any issues. If you have written a unit test for #25, you can check it in. There should be no issues there.

For TOC, I suggest we create a separate issue.

gwierzchowski commented 5 years ago

I did the merges and pushed - what generated new PR (because old was already closed): #36 . Travis tests failed on my test because I suppose #32 is currently not in sambitdash/PDFIO : master (it was merged, but this revert which was later - removed it). I see this looking at Travis record:

PDFIO tests                    |   33      1     34
  Miscellaneous                |    9             9
  Test FlateDecode             |    1             1
  Document without Info        |    1             1
  Document with empty property |           1      1

There is no CDDate which is contained in #32. Will you be able to reapply #32 back to master - this should fix Travis tests for #36? Thanks !

sambitdash commented 5 years ago

I think now this should be fine. Sorry for the confusion. The change history looks very confusing though :-(.

gwierzchowski commented 5 years ago

All looks great now - many Thanks !