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.

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

Path: listdir method to return directory entry names #230

Merged
merged 3 commits into from
Jun 21, 2017

Conversation

stdweird
Copy link
Member

@stdweird stdweird commented Mar 26, 2017

Based on #243

@stdweird stdweird added this to the 17.4 milestone Mar 26, 2017
@stdweird
Copy link
Member Author

Main reason to wrap this is to able to unittest readdir in maven-tools with the mocked files

# Pass NoAction here, as it keeps track of the NoAction value during initialisation and/or keeps_state
my $cafpath = CAF::Path::mkcafpath(log => *$self->{LOG}, NoAction => $options->{noaction});
# only create the directory if it didn't exist yet
# if not, this would for the directory mode on existing directories
Copy link
Contributor

Choose a reason for hiding this comment

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

This line doesn't make sense. "this would [what?] for the"

@stdweird
Copy link
Member Author

@ned21 i'll address the remark, but i made a mistake in the description. this is based on the already merged PR #243; and your remark is from that PR. i'll add a new commit with better description

@ned21
Copy link
Contributor

ned21 commented Jun 17, 2017

@stdweird I thought I had seen that odd sentence before, and assumed I had just reviewed this PR before!

@stdweird
Copy link
Member Author

@ned21 i added a new commit to this PR

Copy link
Member

@jrha jrha left a comment

Choose a reason for hiding this comment

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

LGTM, @ned21?

@stdweird
Copy link
Member Author

retest this please

@ned21 ned21 merged commit 65b2491 into quattor:master Jun 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants