Skip to content
This repository has been archived by the owner on May 20, 2024. It is now read-only.

Don't add component_name if we didn't found one. #1408

Closed
wants to merge 1 commit into from
Closed

Don't add component_name if we didn't found one. #1408

wants to merge 1 commit into from

Conversation

bmillemathias
Copy link

If the expected list of components found is empty, do nothing.
I nonetheless modified also the template so when no component is found
we write it explicitely instead of return 'None'

Fixes #1301

@codecov
Copy link

codecov bot commented Aug 1, 2020

Codecov Report

Merging #1408 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1408      +/-   ##
==========================================
+ Coverage   56.48%   56.49%   +0.01%     
==========================================
  Files          56       56              
  Lines        9143     9143              
==========================================
+ Hits         5164     5165       +1     
+ Misses       3979     3978       -1     

@lgtm-com
Copy link

lgtm-com bot commented Aug 1, 2020

This pull request introduces 1 alert and fixes 3 when merging 0850ba3 into 0fd3f4e - view on LGTM.com

new alerts:

  • 1 for Syntax error

fixed alerts:

  • 2 for Except block handles 'BaseException'
  • 1 for Suspicious unused loop iteration variable

If the expected list of components found is empty, do nothing.
I nonetheless modified also the template so when no component is found
we write it explicitely instead of return 'None'

Fixes #1301
@mkrizek
Copy link
Collaborator

mkrizek commented Aug 4, 2020

Shouldn't we still post the comment suggesting to use the component command when the bot is not able to figure out the component?

I think it makes sense to omit the comment only if the whole component section is missing from the template.

@mkrizek
Copy link
Collaborator

mkrizek commented Sep 18, 2020

@bmillemathias are you still interested on working on this PR?

@mkrizek
Copy link
Collaborator

mkrizek commented Jun 18, 2021

Closing due to inactivity.

@mkrizek mkrizek closed this Jun 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

do not use component template if component is None
2 participants