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 newly-rebased lib into main #856

Merged
merged 9 commits into from
Jan 6, 2023

Conversation

komish
Copy link
Contributor

@komish komish commented Dec 15, 2022

Just three things:

  • Rebased Lib onto Main
  • Fixed a bug in our logging where we were inadvertently not emitting variables because we didn't use sprintf f5852ef
  • Updated the LIBRARY.md doc to comply with the new logging changes. 2899cb0

I left the original lib branch intact so we can use it as a reference, if needed. The first 6 commits here should be aligned with it, barring merge conflicts (which were mostly go mod related)

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 15, 2022
@komish komish requested a review from skattoju December 15, 2022 21:13
@komish
Copy link
Contributor Author

komish commented Dec 15, 2022

golangci-lint was upset about my import ordering so I fixed that and made it its own commit, simply because it touched 17 files 🙄

@coveralls
Copy link

coveralls commented Dec 15, 2022

Coverage Status

Coverage: 81.897% (-2.5%) from 84.438% when pulling e540ad0 on komish:rebasedlib into bdbb21e on redhat-openshift-ecosystem:main.

@acornett21
Copy link
Contributor

/retest

@komish
Copy link
Contributor Author

komish commented Dec 15, 2022

Re E2E Failures: Prow seems to be failing because some logic is creating a directory artifacts at the root of the source, and we already have that directory as a library.

@openshift-ci
Copy link

openshift-ci bot commented Dec 16, 2022

@sebrandon1: changing LGTM is restricted to collaborators

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

.gitignore Outdated Show resolved Hide resolved
@acornett21
Copy link
Contributor

/retest

@acornett21
Copy link
Contributor

/test 4.8-e2e

@acornett21
Copy link
Contributor

/retest

komish and others added 9 commits January 5, 2023 14:48
Signed-off-by: Jose R. Gonzalez <jose@flutes.dev>
Signed-off-by: Jose R. Gonzalez <jose@flutes.dev>
certification/internal was problematic. It cause use to have to
expose in some cases thin "wrappers" that were exported in the right
place in order to be able to access it. Instead, internal allows
the entire module to get to the internal bits.

Signed-off-by: Brad P. Crochet <brad@redhat.com>
Signed-off-by: Jose R. Gonzalez <jose@flutes.dev>
* Moves certification/engine
* Moves certification/pyxis
* Moves certification/runtime
* Moves certification/policy
* Moves certification/artifacts -> artifacts
* Moves certification/formatters -> formatters and internal/formatters
* Leaves the FormatterFunc type exported
* Creates runtime top-level package

Signed-off-by: Brad P. Crochet <brad@redhat.com>
By using logr.Logger, it will allow for a library user or the cli to
inject a logr that conforms to the logr.Logger interface. This frees
up the library from having to do any kind of strange log tricks in
order to handle logging.

Signed-off-by: Brad P. Crochet <brad@redhat.com>
Signed-off-by: Jose R. Gonzalez <jose@flutes.dev>
Signed-off-by: Jose R. Gonzalez <jose@flutes.dev>
Signed-off-by: Jose R. Gonzalez <jose@flutes.dev>
@acornett21
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 5, 2023
@komish komish requested a review from skattoju January 5, 2023 21:26
Copy link
Contributor

@skattoju skattoju left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Jan 6, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: acornett21, komish, sebrandon1, skattoju

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:
  • OWNERS [acornett21,komish,skattoju]

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

@acornett21 acornett21 merged commit 2186885 into redhat-openshift-ecosystem:main Jan 6, 2023
@komish
Copy link
Contributor Author

komish commented Jan 6, 2023

🥳

@sebrandon1
Copy link
Contributor

Great to see this merged into main! Thanks!

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants