Skip to content

Fix wrong logger class in *Cmd.java #5740

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.

Sign up for GitHub

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

Jump to bottom
Merged
rohityadavcloud merged 1 commit into from
Dec 2, 2021
Merged

Fix wrong logger class in *Cmd.java #5740

rohityadavcloud merged 1 commit into from
Dec 2, 2021

Conversation

@davidjumani
Copy link
Contributor

@davidjumani davidjumani commented Dec 1, 2021

Description

This PR changes the incorrect class name used in the logger in ListHypervisorsCmd

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

@davidjumani
Copy link
Contributor Author

davidjumani commented Dec 1, 2021

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Dec 1, 2021

@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

sureshanaparti
sureshanaparti approved these changes Dec 1, 2021
View reviewed changes
Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

good catch, LGTM

@sureshanaparti sureshanaparti added this to the 4.16.1.0 milestone Dec 1, 2021
@davidjumani davidjumani changed the title Fix wrong logger class in ListHypervisorsCmd Fix wrong logger class in *Cmd.java Dec 1, 2021
@blueorangutan
Copy link

blueorangutan commented Dec 1, 2021

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1810

@davidjumani
Copy link
Contributor Author

davidjumani commented Dec 1, 2021

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Dec 1, 2021

@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

blueorangutan commented Dec 1, 2021

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 1812

@davidjumani
Copy link
Contributor Author

davidjumani commented Dec 1, 2021

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Dec 1, 2021

@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

blueorangutan commented Dec 1, 2021

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 1814

@davidjumani
Copy link
Contributor Author

davidjumani commented Dec 2, 2021

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Dec 2, 2021

@davidjumani a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

blueorangutan commented Dec 2, 2021

Trillian Build Failed (tid-2602)

@davidjumani
Copy link
Contributor Author

davidjumani commented Dec 2, 2021

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Dec 2, 2021

@davidjumani a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

rohityadavcloud
rohityadavcloud approved these changes Dec 2, 2021
View reviewed changes
Copy link
Member

@rohityadavcloud rohityadavcloud left a comment

Choose a reason for hiding this comment

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

LGTM - however, the standard practice (that is inconsistent in our codebase) is to pass the class itself and not class's name. For example:

Logger.getLogger(ListTrafficMonitorsCmd.class);

@rohityadavcloud
Copy link
Member

rohityadavcloud commented Dec 2, 2021

Smoketests are not required as changes are in logger only; pkging and travis passed.

@rohityadavcloud rohityadavcloud merged commit dfd75d7 into apache:4.16 Dec 2, 2021
@blueorangutan
Copy link

blueorangutan commented Dec 2, 2021

Trillian test result (tid-2603)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35882 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5740-t2603-kvm-centos7.zip
Smoke tests completed. 91 look OK, 0 have errors
Only failed tests results shown below:

Test Result Time (s) Test File

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

Reviewers

@rohityadavcloud rohityadavcloud rohityadavcloud approved these changes

@harikrishna-patnala harikrishna-patnala harikrishna-patnala approved these changes

@sureshanaparti sureshanaparti sureshanaparti approved these changes

@weizhouapache weizhouapache weizhouapache approved these changes

Assignees

No one assigned

Labels

component:logging

Projects

No open projects
Apache CloudStack 4.16.1
Status: Done

Milestone

4.16.1.0

Development

Successfully merging this pull request may close these issues.

6 participants

@davidjumani @blueorangutan @rohityadavcloud @harikrishna-patnala @sureshanaparti @weizhouapache