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

Merge tag directxman12/v0.4.1 into openshift/master #8

Merged
merged 21 commits into from
Feb 4, 2019

Conversation

mxinden
Copy link

@mxinden mxinden commented Jan 31, 2019

Instead of using the fancy method by @s-urbaniak:

git fetch
git checkout upstream/master
git checkout -b merge
git checkout downstream/master
echo 'merge upstream' | git commit-tree merge^{tree} -p HEAD -p merge
somecommitsha
git checkout somecommitsha

this PR simply does a:

git checkout openshift/master 
git merge v0.4.1

as suggested by @simonpasquier.

Reasoning behind this is that #7 is missing #6 and thereby the Dockerfiles in the root of the directory. Doing a git log --full-history -- Dockerfile reveals that the merge commit itself deletes them.

Do people have thoughts on the way we want to proceed here?

metalmatze and others added 21 commits November 6, 2018 15:57
Update README to container resource metrics API
revert from less ideal wording

more typos
Support custom ca certificate to talk to Prometheus
Add a separate flag for 'start' parameter
Often prometheus is gated by some proxy requiring an auth bearer
token. Currently there is no possibility to configure one except for
providing a full-fledged kubeconfig.

This fixes it by adding a new flag pointing to an optional file
containing the auth bearer for prometheus communication.
cmd/adapter: add prometheus bearer token configuration
Fix empty pod or node list resulting in an error
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 31, 2019
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 31, 2019
@s-urbaniak
Copy link

@mxinden the easier, the better! 👍 Back then the hacky way was necessary as the merge wasn't that easy.

@s-urbaniak
Copy link

s-urbaniak commented Feb 1, 2019

@mxinden now I have finally a bit more air to explain more context.

Note that in the past the merge wasn't easy, hence the above "fancy" method. The above commands simply create a commit object as per https://git-scm.com/book/en/v2/Git-Internals-Git-Objects#_git_commit_objects without any merge logic applied.

You need to make sure yourself though that the tree is properly merged somehow as you recognized.

Back then creating a merge commit manually was simply easier than going through git's internal merge conflict resolution commit by commit. The prerequisite was nevertheless to introduce a proper merge commit manually, which in PR #3 was this commit: c3ac351.

The "fancy" commands just ensure that there is a common commit object pointing to both trees in the commit history retaining original authors and original commit hashes.

also cc @bison fyi

@mxinden
Copy link
Author

mxinden commented Feb 1, 2019

Thanks @s-urbaniak for the details.

Merging this branch locally (git checkout master && git reset --hard openshift/master && git merge merge-directxman12) results in a fast-forward with the below git log preserving both openshift as well as directxman12 git commit sha's.

@brancz would we need an exception to get this merged?

commit 8ab4dc7b78c4376ddb97ef37e5297219c742902f (HEAD -> master, mxinden/merge-directxman12, merge-directxman12)
Merge: 19f9a95 dce283d
Author: Max Leonard Inden <IndenML@gmail.com>
Date:   Thu Jan 31 18:17:31 2019 +0100

    Merge tag 'v0.4.1' into HEAD
    
    v0.4.1

commit 19f9a956abcf670c44b2d49ed90cc337d94b4b9d (openshift/master, openshift/HEAD)
Merge: b1d7720 bb5aea8
Author: Frederic Branczyk <fbranczyk@gmail.com>
Date:   Tue Jan 8 21:45:42 2019 +0100

    Merge pull request #6 from pgier/add-openshift-dockerfile
    
    Add Dockefiles for openshift CI

commit bb5aea8d5551c9a1f6ef6a804766de06a1799222
Author: Paul Gier <pgier@redhat.com>
Date:   Tue Jan 8 14:37:35 2019 -0600

    Add Dockefiles for openshift CI

commit b1d7720326696d6f6c01a67517382dcb97a4be75
Merge: eee898e ae8bab2
Author: Frederic Branczyk <fbranczyk@gmail.com>
Date:   Tue Jan 8 21:23:37 2019 +0100

    Merge pull request #5 from pgier/add-owners-file
    
    add OWNERS file for CI automation

commit ae8bab2228c5ec5615153990508e3aaf4d190d14
Author: Paul Gier <pgier@redhat.com>
Date:   Tue Jan 8 14:21:14 2019 -0600

    add OWNERS file for CI automation

commit dce283def1ad819be55dd2a611f85084e4981313 (tag: v0.4.1, directxman12/master, merge)
Merge: 1e5a868 7624952
Author: Frederic Branczyk <fbranczyk@gmail.com>
Date:   Thu Dec 13 16:03:49 2018 -0800

    Merge pull request #148 from brancz/empty-err
    
    Fix empty pod or node list resulting in an error

commit 76249528703ce650d7d5f596a9418d7a98a4ef86
Author: Frederic Branczyk <fbranczyk@gmail.com>
Date:   Thu Dec 13 15:50:02 2018 -0800

    resourceprovider: Add comments on possible nil results

commit e9ef0bb4d0e28c31ae81f3188b657ce63adf60ee
Author: Frederic Branczyk <fbranczyk@gmail.com>
Date:   Tue Dec 11 10:21:21 2018 -0800

    Fix empty pod or node list resulting in an error

[...]

@simonpasquier
Copy link

When the merge is big or if I have doubts, I compare the upstream tag and the PR like this:

kubernetes-sigs/prometheus-adapter@v0.4.1...mxinden:merge-directxman12

The diff is usually small enough that I can review it and spot merge conflicts that weren't resolved properly.

@brancz
Copy link

brancz commented Feb 1, 2019

As this is a bug fix, no need for exception.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brancz, mxinden

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mxinden
Copy link
Author

mxinden commented Feb 4, 2019

Tested basic kubectl top commands on Openshift 4.0 with quay.io/mxinden/k8s-prometheus-adapter:v0.4.1-openshift.

@mxinden mxinden changed the title [WIP] Merge tag directxman12/v0.4.1 into openshift/master Merge tag directxman12/v0.4.1 into openshift/master Feb 4, 2019
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 4, 2019
@openshift-merge-robot openshift-merge-robot merged commit 815fa76 into openshift:master Feb 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants