Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#225: Fixing incorrect US page size #226

Merged
merged 10 commits into from Aug 6, 2020
Merged

#225: Fixing incorrect US page size #226

merged 10 commits into from Aug 6, 2020

Conversation

Sgkhour
Copy link
Contributor

@Sgkhour Sgkhour commented Jul 28, 2020

corrected the usLetter page height and usLedger page width to reflect 11inx72dpi
closes #225

Sgkhour and others added 2 commits July 28, 2020 09:26
@Sgkhour
Update PDFPageFormat+SizeConstants.swift
c11c368 
corrected the usLetter page height and usLedger page width to reflect 11inx72dpi
Fixed missing file ref
ddfd0ec
@philprime
Copy link
Member

Can you please rebase your branch on the newest develop? I fixed a file reference, so that the unit tests can run again

@Sgkhour
Update PDFPageFormat+SizeConstants.swift
3d0d98b 
corrected the usLetter page height and usLedger page width to reflect 11inx72dpi
@Sgkhour
Merge branch 'develop' of https://github.com/Sgkhour/TPPDF into develop
1c55d9b
@Sgkhour
Update PDFPageFormat+SizeConstants.swift
7cc8344 
corrected the usLetter page height and usLedger page width to reflect 11inx72dpi
@Sgkhour
Merge branch 'develop' of https://github.com/Sgkhour/TPPDF into develop
6d71d24
@Sgkhour
Copy link
Contributor Author

Sgkhour commented Jul 30, 2020

i am afraid there might be something i am not doing right since it seems the continuous integration test keeps failing. i've relied on googling how to do a rebase but maybe that isn't the right way to do it :(

@philprime
Copy link
Member

No, it's alright, you just need to fix the Unit Tests.
They lock down the formats, but as you have changed the format, you also need to change the tests.

Open "TPPDF.xcodeproj" in the root, and you should be able to run the tests from

philprime
philprime reviewed Aug 1, 2020
View reviewed changes
TPPDF.xcodeproj/project.pbxproj Outdated
@@ -21,7 +21,6 @@
/* End PBXAggregateTarget section */

/* Begin PBXBuildFile section */
D477C4C224C45C780097C2AB /* PDFTableCalculatedCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = D477C4C124C45C780097C2AB /* PDFTableCalculatedCell.swift */; };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert removing this, this will exclude the file from the project

philprime
philprime reviewed Aug 1, 2020
View reviewed changes
TPPDF.xcodeproj/project.pbxproj Outdated
@@ -482,7 +481,6 @@
/* End PBXContainerItemProxy section */

/* Begin PBXFileReference section */
D477C4C124C45C780097C2AB /* PDFTableCalculatedCell.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PDFTableCalculatedCell.swift; sourceTree = "<group>"; };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

philprime
philprime reviewed Aug 1, 2020
View reviewed changes
TPPDF.xcodeproj/project.pbxproj Outdated
@@ -1056,7 +1054,6 @@
D4803C3724703E5300DDA039 /* Table */ = {
isa = PBXGroup;
children = (
D477C4C124C45C780097C2AB /* PDFTableCalculatedCell.swift */,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

philprime
philprime reviewed Aug 1, 2020
View reviewed changes
TPPDF.xcodeproj/project.pbxproj Outdated
@@ -2332,7 +2329,6 @@
D4803D1424703E5300DDA039 /* PDFPageLayout.swift in Sources */,
D4803D2024703E5300DDA039 /* PDFTableCellStyle.swift in Sources */,
D4803CFF24703E5300DDA039 /* PDFGenerator+Generation.swift in Sources */,
D477C4C224C45C780097C2AB /* PDFTableCalculatedCell.swift in Sources */,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

@Sgkhour
correct ui unit test
df1c3e1 
reinstate file in unit tests
@Sgkhour
Update project.pbxproj
d05066d
@Sgkhour
Update PDFPageFormat+SizeConstants_Spec.swift
43b7a41 
fixed Pagefornat test definition to the correct usLetter page size
@Sgkhour
Copy link
Contributor Author

Sgkhour commented Aug 1, 2020

thank you for your help.. it turns out i also had to update the test file PDFPageFormat+SizeConstant_Spec.swift with the updated usLetter size info

thanks for your help...
Sami

@philprime
Copy link
Member

Thanks for the changes, can you please undo the changes in the project.pbxproj I commented above, so I can merge the branch afterwards?

@Sgkhour
Copy link
Contributor Author

Sgkhour commented Aug 5, 2020

Philip... if i put those lines back in TPPDF.xcodeproj/project.pbxproj in the develop branch, then the checks fail ... with the error
❌ error: Build input file cannot be found: '/Users/travis/build/techprimate/TPPDF/Source/Internal/Table/PDFTableCalculatedCell.swift' (in target 'TPPDF' from project 'TPPDF')

if i remove them from the project file, then all the test pass. what am i doing wrong ? for now, the lines are back in...

@philprime
Copy link
Member

Alright, I don't get it why it is failing, as the develop branch itself is working (maybe you forked after I fixed that issue).
Doesn't matter, the changes should be fine, so I will merge and close this.

Thanks for your contribution

@philprime philprime merged commit 1788c73 into techprimate:develop Aug 6, 2020
@Sgkhour Sgkhour deleted the develop branch August 24, 2020 00:49
philprime pushed a commit that referenced this pull request Sep 23, 2020
@Sgkhour
Release/2.3.1 (#227)
c82c75c 
* Added macOS compatibility to tests (#224)
* Fixed missing file ref
* #225: Fixing incorrect US page size (#226)
Corrected the usLetter page height and usLedger page width to reflect 11inx72dpi
* Fixed missing file ref
* Updated changelog

Co-authored-by: Sami Khoury <50881610+Sgkhour@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philprime philprime philprime left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Sgkhour @philprime