Skip to content

#130 DirectoryArchiver: create parent directories for symlinks #131

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
Closed
wants to merge 4 commits into from
Closed

#130 DirectoryArchiver: create parent directories for symlinks #131

wants to merge 4 commits into from

Conversation

@jameshans
Copy link
Contributor

@jameshans jameshans commented Dec 14, 2019

No description provided.

@jameshans jameshans mentioned this pull request Dec 14, 2019
Closed
@ghost ghost added the bug label Dec 27, 2019
@ghost ghost added this to the 4.2.2 milestone Dec 27, 2019
@ghost
Copy link

ghost commented Dec 27, 2019

Hi, thanks for the contribution and sorry for the a bit late reply. Looks good to me. If you have time it would be great if you can add test case as well.

ghost
ghost reviewed Feb 2, 2020
View reviewed changes
src/test/java/org/codehaus/plexus/archiver/dir/DirectoryArchiverTest.java Outdated
DirectoryArchiver archiver = new DirectoryArchiver();
archiver.addSymlink( SYMLINK_FILE.getPath(), CURRENT_DIR.getPath() );
archiver.setDestFile( CURRENT_DIR );
archiver.execute();
Copy link

@ghost ghost Feb 2, 2020

Choose a reason for hiding this comment

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

Thanks a lot for the tests. But there is already existing tests for symlinks for dir archivers (sorry forgot to mention it) - org.codehaus.plexus.archiver.SymlinkTest#testSymlinkDirArchiver. Maybe it would be better if this test is added there - to the same method or maybe a new method inside the same test class. I could help with it if you want.

Copy link
Contributor Author

@jameshans jameshans Feb 3, 2020

Choose a reason for hiding this comment

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

Thanks for that, I didn't see the existing tests. That simplifies things.

@ghost
Copy link

ghost commented Feb 3, 2020

Thanks. I've re-based the branch, squashed the commit and did some small formatting change.

@ghost
Copy link

ghost commented Feb 3, 2020

I've expected that having the merge request number in the commit title will be enough to mark it as merged, but unfortunately didn't worked. Merged with 596257f

@ghost ghost closed this Feb 3, 2020
@ghost ghost linked an issue Feb 29, 2020 that may be closed by this pull request
DirectoryArchiver fails for symlinks if a parent directory doesn't exist #130
Closed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug

Projects

None yet

Milestone

4.2.2

Development

Successfully merging this pull request may close these issues.

DirectoryArchiver fails for symlinks if a parent directory doesn't exist

1 participant

@jameshans