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

When directory is specified, danger-swiftlint is still linting files outside the specified directory #53

Merged
merged 3 commits into from
Sep 8, 2017

Conversation

victorg1991
Copy link
Collaborator

Right now, when I run danger-swiftlint with a custom directory and I have also modified files outside this directory, danger-swiftlint is linting all the files.

With this PR danger-swiftlint will only lint the modified files inside the specified directory :)

If this PR gets approved, can you upload a new version of the Gem? Thanks in advance!

Any comment or question would be appreciated

@victorg1991 victorg1991 requested a review from thii September 3, 2017 12:13
Copy link
Collaborator

@thii thii left a comment

Choose a reason for hiding this comment

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

LGTM w/ nits.

@@ -113,6 +115,8 @@ def find_swift_files(files=nil, excluded_paths=[])
# Remove dups
uniq.
map { |file| File.expand_path(file) }.
# Ensure only files in the selected directory
select { |file| file.start_with?(dir_selected)}.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't there be a space before }?


@swiftlint.lint_files
end

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like indents are somewhat incorrect in this chunk.

@victorg1991
Copy link
Collaborator Author

I've addressed the comments @thii :)

Do you have permissions to upload the gem?

@thii
Copy link
Collaborator

thii commented Sep 3, 2017

@victorg1991 I don't :)
However, you can point your Gemfile to this repository in the mean time.

@victorg1991
Copy link
Collaborator Author

@thii even if I'm using this in danger?

@thii
Copy link
Collaborator

thii commented Sep 3, 2017

@victorg1991 Yes. Danger plugins are just normal Ruby gems.

@victorg1991
Copy link
Collaborator Author

@ashfurrow any chance to take a look into this? :)

Thanks!

@ashfurrow
Copy link
Owner

Ah, sorry for the delay. Will look into it now.

Copy link
Owner

@ashfurrow ashfurrow left a comment

Choose a reason for hiding this comment

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

Looks good, I'll add a changelog entry after merging 👍

@ashfurrow ashfurrow merged commit f1ce5e3 into master Sep 8, 2017
@ashfurrow ashfurrow deleted the custom-dir-bug branch September 8, 2017 20:34
@ashfurrow
Copy link
Owner

Cool, this has been released as 0.9.0. Thanks again!

@victorg1991
Copy link
Collaborator Author

Wow so fast, thank you so much : D

@ashfurrow
Copy link
Owner

No worries! Feel free to ping me if I ever forget something 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants