Closed MartinLau7 closed 5 years ago
First of all, I am sorry that I have not made the correct modifications in the submitted code, I will improve these errors in the future.
Hey Martin, no big deal, this is part of the process for a pull request. Let's work on this together. The PR isn't final - you can push more changes to your branch, and it'll update.
@abbeycode There is currently a change in the package, I seem to be destroying the version of Xcode, it has been upgraded to Xcode 10.2
Please let me know when you're done with your changes and are ready for me to review again. Also, make sure you review your changes in the "Files changed" tab when you're done, so they only contain changes you intended to make (not making whitespace changes to code you otherwise didn't touch).
Also, if you're having trouble understanding any of my comments, please let me know, and I can try to rephrase, or go into more detail.
I just saw your Xcode 10.2 comment. The version of Xcode you use shouldn't matter, so long as you don't commit any upgrade checks or use APIs only available in newer versions of the frameworks (which Xcode shouldn't let you do unless you change the deployment target, which you shouldn't change either :)
Did you see the list of requests I made in the initial review comment?
Did you see the list of requests I made in the initial review comment?
Yes, I see this problem now, because I have an exception in the code version of the workbench that I can't resubmit, I need to close the Pull Request later, until I rearrange and create a new Pull Request. What do you think?
You don't need to close this PR, and you don't need to use a different version of Xcode. Just back out the changes you made to the project.pbxproj
and UnzipKit.xcscheme
files that aren't directly necessary for your change. You can use Xcode to this, if you view the changes from the v1.9 branch. Once you've backed out those changes, you can push to this branch again, and the PR will update.
You don't need to close this PR, and you don't need to use a different version of Xcode. Just back out the changes you made to the
project.pbxproj
andUnzipKit.xcscheme
files that aren't directly necessary for your change. You can use Xcode to this, if you view the changes from the v1.9 branch. Once you've backed out those changes, you can push to this branch again, and the PR will update.
@abbeycode Since Xcode breaks git, it is not considered to turn off PR before. Now I have resubmitted and passed the Travis CI build, I will continue to modify if there are new issues.
I've created a new branch, pr84
, to merge your PR into, for me to then work on it further. Can you please rebase your changes on that branch? I merged a different PR just now, that made two single-line fixes. After your rebase I'll merge into that branch and create a new PR for you to review once I've implemented the changes we discussed. You can keep the "v1.9" branch name in your repo. That shouldn't matter.
I've created a new branch,
pr84
, to merge your PR into, for me to then work on it further. Can you please rebase your changes on that branch? I merged a different PR just now, that made two single-line fixes. After your rebase I'll merge into that branch and create a new PR for you to review once I've implemented the changes we discussed. You can keep the "v1.9" branch name in your repo. That shouldn't matter.
I have rebase to "PR84", but there are some merge errors, I think I need to modify it in my free time, I will add some new code to optimize the "UZKFileInfo - isDirectory" implementation, then I will commit again.
What are you changing in the isDirectory
implementation?
Also, the changes I'm asking you to rebase into your branch are super simple: just a single commit affecting two lines (link here). Just resolve the rebase conflict using your version of UZKArchive.m
, and before you commit make that same change, replacing UZKCompressionMethodDefault
with method
on those two lines.
I have rebased to "PR84" and modified the previous error. In the new commit I modified the isDirectory code to make it better for type judgment, and I added "isSymLink" and "isResourceFork". "isSymLink" can determine if the file is a symbolic link. "isResourceFork" can help us decide whether to decompress the "__MACOSX" content.
The new commit seems to have changed something in Xcode, and I have avoided submitting Xcode files as much as possible. This makes me very annoyed, I need your help to get it back to the original version.
Martin, I would like to merge this PR into the pr84
branch I created and make the remaining changes I'd like to see there myself. What would you like to get back to the original version, and what do you mean by the original version? There are ways in Git and in Xcode to checkout an older revision of a file over your working copy (or to compare its contents to the current revision), and then you can use Xcode to select which changes you want to commit (if any).
I'm going to squash and merge your PR and will start making my own commits on top. I'll open up a new PR and we can continue the discussion there.
PR #87 was merged into the v1.9 branch, including these changes
Submit log
Test Code