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

commonAddedLinesContains #271

Closed
steprescott opened this issue Apr 6, 2020 · 6 comments
Closed

commonAddedLinesContains #271

steprescott opened this issue Apr 6, 2020 · 6 comments

Comments

@steprescott
Copy link

steprescott commented Apr 6, 2020

I would like to check each line of any added or modified file to see if it contains a http:// url. However I can't seem to get anything to be flagged up as a warning. I'm currently on 1.22.1 as Peril isn't updated to danger 10.0.0 yet.

Although my regex isn't pro level I have checked using online testers. Is there anything below that you see I'm doing wrong?

import { commonAddedLinesContains } from 'danger-plugin-toolbox'

commonAddedLinesContains(/.*/, /http:\/\//i, file => `The file "${file}" contains insecure URL.`);

I've also checked to see if the job, due to it being async, needed to be scheduled.
Note that the messages are added to the PR

import { danger, schedule, message, warn } from 'danger';
import { commonAddedLinesContains } from 'danger-plugin-toolbox'

schedule(async () => {
  message('Before');
  await commonAddedLinesContains(/.*/, /http:\/\//i, file => `The file "${file}" contains insecure URL.`);
  message('After');
});

Thanks in advance.

@sogame
Copy link
Owner

sogame commented Apr 6, 2020

Hi @steprescott,

It's really weird, I've just created this PR and it seems to be working fine: #272

Might it be related to Peril?
commonAddedLinesContains uses danger.git.diffForFile, and I've seen this issue: danger/peril#454

Could you do a couple of checks, please?

  • Log the modified files, to make sure they're the expected ones: message(JSON.stringify(committedFilesGrep(/.*/)));
  • Log the output of diffForFile for one of the modified files, to check if the output is the expected one or null like in the mentioned issue.

Thanks!

@sogame
Copy link
Owner

sogame commented Apr 6, 2020

Oh, about using 1.22.1, I think you should be able to upgrade, danger is not a dependency of danger-plugin-toolbox, it's just installed to run danger in pr's (although upgrading won't fix this issue 😅)

@steprescott
Copy link
Author

Thanks for the quick reply and sorry for my slow response. I'll log out what you suggested and then will post it back here. Thanks for the support. I'm glad the regex is working 😄

@sogame
Copy link
Owner

sogame commented Apr 28, 2020

Hi @steprescott, any progress so far? 😇

@steprescott
Copy link
Author

Sorry, I've been pulled in different directions so not getting a lot of time to look at this. Happy to close the ticket and reopen if it still occurs when I get back onto it.

@sogame
Copy link
Owner

sogame commented Apr 28, 2020

Was more curiosity than concern about having the issue open 😂
Let me know any progress, happy to try to help if possible :-)

@sogame sogame closed this as completed Apr 28, 2020
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

No branches or pull requests

2 participants