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 scale_statefulset activity #73

Merged

Conversation

tdevilleduc
Copy link
Contributor

Signed-off-by: Thomas DEVILLE-DUC thomas.deville-duc-prestataire@laposte.fr

Copy link
Contributor

@Lawouach Lawouach left a comment

Choose a reason for hiding this comment

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

LGTM

Just lacks a changelog entry :)

@Lawouach
Copy link
Contributor

Lawouach commented Jan 8, 2020

Looks good, is this replacing one of the oldest PRs? if so, could you close the ones it supersedes please?

@tdevilleduc
Copy link
Contributor Author

Just lacks a changelog entry :)

Sorry that's my first PRs. I'm on it

@tdevilleduc
Copy link
Contributor Author

Looks good, is this replacing one of the oldest PRs? if so, could you close the ones it supersedes please?

I cut #68 in two different PRs : 68 and 73. But they both do something.

@codecov-io
Copy link

codecov-io commented Jan 9, 2020

Codecov Report

Merging #73 into master will increase coverage by 0.21%.
The diff coverage is 85.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
+ Coverage   80.05%   80.26%   +0.21%     
==========================================
  Files           4        5       +1     
  Lines         361      375      +14     
==========================================
+ Hits          289      301      +12     
- Misses         72       74       +2
Impacted Files Coverage Δ
chaosk8s/statefulset/actions.py 85.71% <85.71%> (ø)

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 ad79c04...fffa570. Read the comment docs.

Copy link
Contributor

@Lawouach Lawouach left a comment

Choose a reason for hiding this comment

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

LGTM

@tdevilleduc tdevilleduc force-pushed the scale_statefulset_activity branch 2 times, most recently from a2eb143 to fffa570 Compare January 9, 2020 13:17
@Lawouach
Copy link
Contributor

Lawouach commented Jan 9, 2020

did you merge commit or rebase from master? It's surprising to see this commit here 0829553

@tdevilleduc
Copy link
Contributor Author

did you merge commit or rebase from master? It's surprising to see this commit here 0829553

I feel the same.
I merged master from here to my master branch. Then i rebased my branch with my master
But I don't really understand how to synchronize my fork repo with this one

@Lawouach
Copy link
Contributor

Lawouach commented Jan 9, 2020

Oh I see.

Don't worry, I'm not a git guru either :)

To merge upstream into your master (or actually even directly to your branch).

$ git remote add upstream https://github.com/chaostoolkit/chaostoolkit-kubernetes.git
$ git pull --rebase origin upstream

The idea is that you configure upstream to point to the master from this repository. Then you pull and rebase directly from upstream master into your branch (without going through your local master).

Signed-off-by: Thomas DEVILLE-DUC <thomas.deville-duc-prestataire@laposte.fr>
@tdevilleduc
Copy link
Contributor Author

With your help, i think i manage
I did (not exactly the same) :
$ git remote add upstream https://github.com/chaostoolkit/chaostoolkit-kubernetes.git
$ git pull --rebase upstream master
$ git push --force

@Lawouach Lawouach merged commit fe896df into chaostoolkit:master Jan 9, 2020
@Lawouach
Copy link
Contributor

Lawouach commented Jan 9, 2020

now, let's pull rebase the next PR :)

@tdevilleduc tdevilleduc deleted the scale_statefulset_activity branch January 9, 2020 13:55
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.

3 participants