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

Add timestampFlag docs #997

Merged
merged 14 commits into from Jan 30, 2020
Merged

Add timestampFlag docs #997

merged 14 commits into from Jan 30, 2020

Conversation

drov0
Copy link
Contributor

@drov0 drov0 commented Dec 18, 2019

Reason for this PR is that it makes the CI fail until #987 is merged

Reason for this PR is that it makes the CI fail until urfave#987 is merged
@drov0 drov0 requested a review from a team as a code owner December 18, 2019 16:35
@drov0 drov0 mentioned this pull request Dec 18, 2019
docs/v2/manual.md Outdated Show resolved Hide resolved
coilysiren
coilysiren previously approved these changes Dec 19, 2019
@coilysiren coilysiren requested review from a team and saschagrunert and removed request for a team December 19, 2019 00:01
Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

I'll approve if the CI is working and the related PR got merged :)

@asahasrabuddhe
Copy link
Member

This won't pass until the changes that have been merged are tagged. When gfmrun tries to execute the code, it pulls the v2.0.0 which currently does not have this change. I am thinking of a way to fix this once and for all because everytime we have a new feature, this problem is going to recur.

Now we have two ways to move forward, make a release with a feature without having the documentation supporting the feature or, to just merge this as it's only documentation and we all know the exact reason for the CI failure.

@coilysiren
Copy link
Member

I am thinking of a way to fix this once and for all because everytime we have a new feature, this problem is going to recur.

From a functionality point of view, this is what I would like => #1006

make a release with a feature without having the documentation supporting the feature

This is my preference for the time being 👍

@codecov
Copy link

codecov bot commented Jan 26, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@21ae70f). Click here to learn what that means.
The diff coverage is 61.53%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #997   +/-   ##
=========================================
  Coverage          ?   72.83%           
=========================================
  Files             ?       33           
  Lines             ?     2474           
  Branches          ?        0           
=========================================
  Hits              ?     1802           
  Misses            ?      565           
  Partials          ?      107
Impacted Files Coverage Δ
app.go 78.71% <ø> (ø)
altsrc/flag_generated.go 64.28% <ø> (ø)
context.go 95.45% <ø> (ø)
altsrc/map_input_source.go 52.85% <ø> (ø)
altsrc/json_source_context.go 19.83% <0%> (ø)
flag.go 86.22% <100%> (ø)
flag_string.go 88.88% <100%> (ø)
flag_duration.go 70% <100%> (ø)
flag_int_slice.go 74.62% <100%> (ø)
flag_string_slice.go 79.66% <100%> (ø)
... and 11 more

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 21ae70f...8a9326b. Read the comment docs.

@asahasrabuddhe
Copy link
Member

Finally this works! Phew

Copy link
Member

@saschagrunert saschagrunert left a comment

Choose a reason for hiding this comment

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

Hey 👋, I just found some formatting nits. 👇

docs/v2/manual.md Outdated Show resolved Hide resolved
docs/v2/manual.md Outdated Show resolved Hide resolved
docs/v2/manual.md Outdated Show resolved Hide resolved
formatting nits
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

Successfully merging this pull request may close these issues.

None yet

4 participants