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

Remove invalid URI warning #94

Merged
merged 1 commit into from
Sep 16, 2020
Merged

Conversation

JShep1
Copy link

@JShep1 JShep1 commented Sep 16, 2020

Ever since the addition of fuel world support, we've started to check the passed in file string. In order to check for a valid URI link, we've been using common::URI (here) which prints a warning message if the URI is not a correctly formatted URI. Since we are using this class to check for a valid URI, the warning message removed in this PR prints for every non-valid URI file which is quite misleading.

Signed-off-by: John Shepherd [email protected]

Signed-off-by: John Shepherd <[email protected]>
@JShep1 JShep1 requested a review from chapulina September 16, 2020 00:30
@JShep1 JShep1 requested a review from mjcarroll as a code owner September 16, 2020 00:30
@github-actions github-actions bot added 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels Sep 16, 2020
@codecov
Copy link

codecov bot commented Sep 16, 2020

Codecov Report

Merging #94 into ign-common3 will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-common3      #94      +/-   ##
===============================================
- Coverage        73.94%   73.94%   -0.01%     
===============================================
  Files               69       69              
  Lines             9391     9390       -1     
===============================================
- Hits              6944     6943       -1     
  Misses            2447     2447              
Impacted Files Coverage Δ
src/URI.cc 98.71% <100.00%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81ba1c9...47a8ee3. Read the comment docs.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

Yes! The caller can always call URI::Valid and print a warning if they need to.

@JShep1 JShep1 merged commit 6f82315 into ign-common3 Sep 16, 2020
@JShep1 JShep1 deleted the jshep1/remove-uri-warning branch September 16, 2020 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants