Skip to content

Support full file issues reported at line 0#238

Merged
La0 merged 4 commits intomozilla:masterfrom
La0:fix-237
Nov 15, 2019
Merged

Support full file issues reported at line 0#238
La0 merged 4 commits intomozilla:masterfrom
La0:fix-237

Conversation

@La0
Copy link
Copy Markdown
Collaborator

@La0 La0 commented Nov 15, 2019

With this code, the licence checks are now reported: Example on phab-dev

The coverage issue are now also considered full file issues (which makes sense too)

Fixes #230

@La0 La0 added bug Something isn't working bot Code review bot labels Nov 15, 2019
@La0 La0 requested a review from marco-c November 15, 2019 08:22
@La0 La0 self-assigned this Nov 15, 2019
@marco-c
Copy link
Copy Markdown
Collaborator

marco-c commented Nov 15, 2019

We could instead make the license analyzer follow our JSON standard and make it return None instead of 0 as the line

@marco-c
Copy link
Copy Markdown
Collaborator

marco-c commented Nov 15, 2019

We could instead make the license analyzer follow our JSON standard and make it return None instead of 0 as the line

As discussed on IRC, let's support 0 as None, but let's log an error on Sentry so we can fix analyzers that use 0 to mean full file to use None instead, and analyzers who use 0 as first line to add +1 to their lines.

@La0
Copy link
Copy Markdown
Collaborator Author

La0 commented Nov 15, 2019

@marco-c I updated the bot's code to send a warning on line 0, and falling back to full file mode

@La0 La0 merged commit c4e37b5 into mozilla:master Nov 15, 2019
@La0 La0 deleted the fix-237 branch November 15, 2019 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot Code review bot bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Negative line in issues crash the bot

2 participants