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

fix(doc strings): Fix bug related documentation/clean up of default configurations #2331 #2388

Merged
merged 3 commits into from
Mar 8, 2020

Conversation

NikeNano
Copy link
Contributor

@NikeNano NikeNano commented Mar 8, 2020

Bug fix for the last comments here #2331 (review) which were not fixed before master.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I have written unit and/or e2e tests for my change. PRs without these are unlike to be merged.
  • Optional. I've added My organization is added to the README.
  • I've signed the CLA and required builds are green.

@NikeNano
Copy link
Contributor Author

NikeNano commented Mar 8, 2020

FYI simster7 PR with your requested changes from #2331

Copy link
Member

@simster7 simster7 left a comment

Choose a reason for hiding this comment

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

Thanks @NikeNano! One quick change needed

workflow/controller/controller.go Outdated Show resolved Hide resolved
Copy link
Member

@simster7 simster7 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 @NikeNano! Can I encourage you to look at our help wanted issues and keep contributing? 😄

@NikeNano
Copy link
Contributor Author

NikeNano commented Mar 8, 2020

LGTM, thanks @NikeNano! Can I encourage you to look at our help wanted issues and keep contributing? 😄

Yeah, I will continue to contribute :)

Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Mar 8, 2020

Codecov Report

Merging #2388 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2388   +/-   ##
=======================================
  Coverage   13.13%   13.13%           
=======================================
  Files          72       72           
  Lines       25084    25084           
=======================================
  Hits         3294     3294           
  Misses      21375    21375           
  Partials      415      415
Impacted Files Coverage Δ
workflow/config/config.go 33.33% <ø> (ø) ⬆️
workflow/controller/controller.go 2.5% <ø> (ø) ⬆️

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 77e11fc...4207a30. Read the comment docs.

@alexec alexec merged commit 1d21d3f into argoproj:master Mar 8, 2020
@alexec
Copy link
Contributor

alexec commented Mar 8, 2020

I think we should have a page under /docs telling people how to use this and provide some examples.

@simster7
Copy link
Member

simster7 commented Mar 8, 2020

I think we should have a page under /docs telling people how to use this and provide some examples.

Agreed... @NikeNano would you own this?

@NikeNano
Copy link
Contributor Author

NikeNano commented Mar 8, 2020

I think we should have a page under /docs telling people how to use this and provide some examples.

Agreed... @NikeNano would you own this?

Sure, I will work on this!

@alexec
Copy link
Contributor

alexec commented Mar 8, 2020

Great! Docs will encourage people to try the feature out. It might be worth while adding stability icon (e.g. alpha) and version (v2.7), example:

https://github.com/argoproj/argo/blob/master/docs/workflow-archive.md

alexec pushed a commit that referenced this pull request Mar 13, 2020
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.

4 participants