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

refactor log handling #381

Merged
merged 2 commits into from
May 14, 2020
Merged

refactor log handling #381

merged 2 commits into from
May 14, 2020

Conversation

jkhelil
Copy link

@jkhelil jkhelil commented May 12, 2020

Changes

I propose avoid passing logger as argument in function signature because it is confusing, adding complexity.
we can either add and init the logger in the struct object in order to use it on underlying method or just add it in method

for example:
return &ReconcileJenkinsBaseConfiguration{
Configuration: config,
logger: log.Log.WithValues("cr", config.Jenkins.Name),
jenkinsAPIConnectionSettings: jenkinsAPIConnectionSettings,
}
or here:
func (r *ReconcileJenkins) Reconcile(request reconcile.Request) (reconcile.Result, error) {
reconcileFailLimit := uint64(10)
logger := logx.WithValues("cr", request.Name)

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide for more details.

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS.

Release Notes

Describe any user facing changes here, or delete this block.

Examples of user facing changes:
- API changes
- Bug fixes
- Any changes in behavior

pkg/controller/jenkins/jenkins_controller.go Outdated Show resolved Hide resolved
pkg/controller/jenkins/jenkins_controller.go Outdated Show resolved Hide resolved
@tomaszsek tomaszsek changed the title refactor log handeling refactor log handling May 13, 2020
pkg/controller/jenkins/jenkins_controller.go Outdated Show resolved Hide resolved
pkg/controller/jenkins/jenkins_controller.go Outdated Show resolved Hide resolved
@tomaszsek tomaszsek merged commit 2dea712 into jenkinsci:master May 14, 2020
@tomaszsek tomaszsek added the bug Something isn't working label Jan 18, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants