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.

Sign up for GitHub

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

Jump to bottom

Add StatusText field #1649

Merged
merged 5 commits into from
Oct 12, 2020
Merged

Add StatusText field #1649

merged 5 commits into from
Oct 12, 2020

Conversation

lcd1232
Copy link
Contributor

@lcd1232 lcd1232 commented Oct 4, 2020

Closes #1585

@CLAassistant
Copy link

CLAassistant commented Oct 4, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@codecov-commenter
Copy link

codecov-commenter commented Oct 4, 2020 •
edited
Loading

Codecov Report

Merging #1649 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1649   +/-   ##
=======================================
  Coverage   72.16%   72.16%           
=======================================
  Files         166      166           
  Lines       12787    12788    +1     
=======================================
+ Hits         9228     9229    +1     
  Misses       3009     3009           
  Partials      550      550
Impacted Files Coverage Δ
lib/netext/httpext/response.go 91.11% <ø> (ø)
lib/netext/httpext/request.go 96.93% <100.00%> (+0.01%) ⬆️
lib/executors.go 90.80% <0.00%> (-2.30%) ⬇️
js/runner.go 83.01% <0.00%> (+0.75%) ⬆️

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 9b75efd...8d5e6d5. Read the comment docs.

@lcd1232
Copy link
Contributor Author

lcd1232 commented Oct 4, 2020

@mstoykov could you add the topic hacktoberfest ? Right now PR has status as "Ineligible Repository"
image

@mstoykov
Copy link
Collaborator

mstoykov commented Oct 5, 2020

@lcd1232 I added it :)

imiric
imiric suggested changes Oct 5, 2020
View reviewed changes
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

Hi, thanks for your contribution. Would you mind adding unit tests for this change?

@codecov-io
Copy link

codecov-io commented Oct 11, 2020 •
edited
Loading

Codecov Report

Merging #1649 into master will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1649      +/-   ##
==========================================
- Coverage   72.16%   72.15%   -0.02%     
==========================================
  Files         166      166              
  Lines       12787    12788       +1     
==========================================
- Hits         9228     9227       -1     
- Misses       3009     3011       +2     
  Partials      550      550
Impacted Files Coverage Δ
lib/netext/httpext/response.go 91.11% <ø> (ø)
lib/netext/httpext/request.go 96.93% <100.00%> (+0.01%) ⬆️
lib/testutils/minirunner/minirunner.go 81.39% <0.00%> (-4.66%) ⬇️
lib/executors.go 90.80% <0.00%> (-2.30%) ⬇️
lib/executor/vu_handle.go 95.23% <0.00%> (+1.90%) ⬆️

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 9b75efd...c9940c5. Read the comment docs.

@lcd1232
Copy link
Contributor Author

lcd1232 commented Oct 11, 2020

@imiric I fixed linter and add tests. Could you review, please?

mstoykov
mstoykov previously approved these changes Oct 12, 2020
View reviewed changes
@mstoykov mstoykov requested a review from imiric October 12, 2020 08:16
na--
na-- previously approved these changes Oct 12, 2020
View reviewed changes
@na-- na-- added this to the v0.29.0 milestone Oct 12, 2020
imiric
imiric previously approved these changes Oct 12, 2020
View reviewed changes
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the test!

lib/netext/httpext/request_test.go Outdated
Comment on lines 167 to 168
//_, err := w.Write([]byte("some body"))
//require.NoError(t, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@lcd1232 lcd1232 dismissed stale reviews from imiric, na--, and mstoykov via c9940c5 October 12, 2020 10:15
@mstoykov mstoykov merged commit 8786dfb into grafana:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mstoykov mstoykov mstoykov approved these changes

@imiric imiric imiric left review comments

@na-- na-- na-- left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.29.0
Development

Successfully merging this pull request may close these issues.

Export the status line text in HTTP response objects
7 participants
@lcd1232 @CLAassistant @codecov-commenter @mstoykov @codecov-io @imiric @na--