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

First tools/ refactor for review #1913

Merged
merged 1 commit into from Nov 10, 2017

Conversation

3 participants
@AustinReichert
Contributor

AustinReichert commented Nov 9, 2017

@brauner Here's the first refactor in the tools directory for you to review

Signed-off-by: Austin Reichert austinskyreichert@utexas.edu

@lxc-jenkins

This comment has been minimized.

Show comment
Hide comment
@lxc-jenkins

lxc-jenkins Nov 9, 2017

This pull request didn't trigger Jenkins as its author isn't in the whitelist.

An organization member must perform one of the following:

  • To have this branch tested by Jenkins, use the "ok to test" command.
  • To have a one time test done, use the "test this please" command.

Those commands are simple Github comments of the format: "jenkins: COMMAND"

lxc-jenkins commented Nov 9, 2017

This pull request didn't trigger Jenkins as its author isn't in the whitelist.

An organization member must perform one of the following:

  • To have this branch tested by Jenkins, use the "ok to test" command.
  • To have a one time test done, use the "test this please" command.

Those commands are simple Github comments of the format: "jenkins: COMMAND"

Show outdated Hide outdated src/lxc/tools/lxc_attach.c Outdated
@brauner

This comment has been minimized.

Show comment
Hide comment
@brauner

brauner Nov 9, 2017

Member

I'll nitpick a little at the beginning as this might be helpful later on. Can you please squash the two commits together. In case you're unsure how to do this here are some instructions. In your branch do:

git rebase -S -i HEAD~2

And when you are dropped in interactive mode you will see a list of the commits like this:

pick c8c35370 commands: clear console log file
pick 4cca3369 console: add comment about log file

# Rebase 29e4eb31..4cca3369 onto 29e4eb31 (10 commands)
#
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
# f, fixup = like "squash", but discard this commit's log message
# x, exec = run command (the rest of the line) using shell
# d, drop = remove commit
#
# These lines can be re-ordered; they are executed from top to bottom.
#
# If you remove a line here THAT COMMIT WILL BE LOST.
#
# However, if you remove everything, the rebase will be aborted.
#
# Note that empty commits are commented out

Now replace the pick of the latest commit with squash:

pick c8c35370 commands: clear console log file
squash 4cca3369 console: add comment about log file

Then you'll be dropped into another prompt where you can edit the commit message. Please replace the commit message with something like this:

tools/lxc-attach: remove internal logging
Signed-off-by: Austin Reichert austinskyreichert@utexas.edu

The idea is to always have something like

<module>: <minimal descriptive 75/80 char message>
<sign-off>
Member

brauner commented Nov 9, 2017

I'll nitpick a little at the beginning as this might be helpful later on. Can you please squash the two commits together. In case you're unsure how to do this here are some instructions. In your branch do:

git rebase -S -i HEAD~2

And when you are dropped in interactive mode you will see a list of the commits like this:

pick c8c35370 commands: clear console log file
pick 4cca3369 console: add comment about log file

# Rebase 29e4eb31..4cca3369 onto 29e4eb31 (10 commands)
#
# Commands:
# p, pick = use commit
# r, reword = use commit, but edit the commit message
# e, edit = use commit, but stop for amending
# s, squash = use commit, but meld into previous commit
# f, fixup = like "squash", but discard this commit's log message
# x, exec = run command (the rest of the line) using shell
# d, drop = remove commit
#
# These lines can be re-ordered; they are executed from top to bottom.
#
# If you remove a line here THAT COMMIT WILL BE LOST.
#
# However, if you remove everything, the rebase will be aborted.
#
# Note that empty commits are commented out

Now replace the pick of the latest commit with squash:

pick c8c35370 commands: clear console log file
squash 4cca3369 console: add comment about log file

Then you'll be dropped into another prompt where you can edit the commit message. Please replace the commit message with something like this:

tools/lxc-attach: remove internal logging
Signed-off-by: Austin Reichert austinskyreichert@utexas.edu

The idea is to always have something like

<module>: <minimal descriptive 75/80 char message>
<sign-off>
@AustinReichert

This comment has been minimized.

Show comment
Hide comment
@AustinReichert

AustinReichert Nov 9, 2017

Contributor

No worries, I could use some work on my proper git skills/conventions. I'll fix it.

Contributor

AustinReichert commented Nov 9, 2017

No worries, I could use some work on my proper git skills/conventions. I'll fix it.

tools/lxc_attach: removed api logging
Signed-off-by: Austin Reichert <austinskyreichert@utexas.edu>
@brauner

This comment has been minimized.

Show comment
Hide comment
@brauner

brauner Nov 10, 2017

Member

jenkins: test this please

Member

brauner commented Nov 10, 2017

jenkins: test this please

@brauner brauner merged commit c3b3643 into lxc:master Nov 10, 2017

4 checks passed

Branch target Branch target is correct
Details
Signed-off-by All commits signed-off
Details
Testsuite Testsuite passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@brauner

This comment has been minimized.

Show comment
Hide comment
@brauner

brauner Nov 10, 2017

Member

Thanks.

Member

brauner commented Nov 10, 2017

Thanks.

@AustinReichert AustinReichert deleted the AustinReichert:remove-api-calls branch Nov 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment