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.

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

NMS-16986: System Check Utility : Basic Collection #7587

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from
Open

NMS-16986: System Check Utility : Basic Collection #7587

wants to merge 22 commits into from

Conversation

mwajahatabbasi
Copy link

@mwajahatabbasi mwajahatabbasi commented Jan 7, 2025 •
edited
Loading

All Contributors

Contribution Checklist

  • Please make an issue in the OpenNMS issue tracker if there isn't one already.
    Once there is an issue, please:
    1. update the title of this PR to be in the format: ${JIRA-ISSUE-NUMBER}: subject of pull request
    2. update the Jira link at the bottom of this comment to refer to the real issue number
    3. prefix your commit messages with the issue number, if possible
    4. once you've created this PR, please link to it in a comment in the Jira issue
      Don't worry if this sounds like a lot, we can help you get things set up properly.
  • If this code is likely to affect the UI, did you name your branch with -smoke in it to trigger smoke tests?
  • If this is a new or updated feature, is there documentation for the new behavior?
  • If this is new code, are there unit and/or integration tests?
  • If this PR targets a foundation-* branch, does it try to avoid changing files in $OPENNMS_HOME/etc/?

What's Next?

A PR should be assigned at least 2 reviewers. If you know that someone would be a good person to review your code, feel free to add them.

If you need help making additions or changes to the documentation related to your changes, please let us know.

In any case, if anything is unclear or you want help getting your PR ready for merge, please don't hesitate to say something in the comments here,
or in the #opennms-development chat channel.

Once reviewer(s) accept the PR and the branch passes continuous integration, the PR is eligible for merge.

At that time, if you have commit access (are an OpenNMS Group employee or a member of the OGP) you are welcome to merge the PR when you're ready.
Otherwise, a reviewer can merge it for you.

Thanks for taking time to contribute!

External References

@mwajahatabbasi mwajahatabbasi changed the title NMS-16986 : Added missing items as required in ticket. NMS-16986 : System Check Utility : Basic Collection Jan 7, 2025
@mwajahatabbasi mwajahatabbasi marked this pull request as draft January 7, 2025 18:46
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@indigo423 indigo423 changed the title NMS-16986 : System Check Utility : Basic Collection NMS-16986: System Check Utility : Basic Collection Jan 9, 2025
@mwajahatabbasi mwajahatabbasi marked this pull request as ready for review January 16, 2025 10:24
Junaid-Khan-Nant
Junaid-Khan-Nant reviewed Jan 16, 2025
View reviewed changes
features/system-report/src/main/java/org/opennms/systemreport/system/HardDriveReportPlugin.java Show resolved Hide resolved
features/system-report/src/main/java/org/opennms/systemreport/AbstractSystemReportPlugin.java Outdated
while (br.ready()) {
String line=null;
while ((line = br.readLine()) != null) { //br.ready() not working as desired so code modified.
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented code.

naeem-afzal
naeem-afzal reviewed Jan 17, 2025
View reviewed changes
Copy link

@naeem-afzal naeem-afzal left a comment

Choose a reason for hiding this comment

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

LGTM

@mwajahatabbasi
Merge pull request #7594 from OpenNMS/Jira/NMS-17002-grouping-system-…
edf0c3f 
…reports

FE: NMS-17002: System Check Utility : Grouping of System Report at front end.
@mwajahatabbasi
NMS-16986 : Handel Merge changes as well as changes suggest in standu…
5d696cd 
…p meeting.
cgorantla
cgorantla reviewed Jan 29, 2025
View reviewed changes
Copy link
Contributor

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

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

Just one comment, otherwise LGTM!

features/system-report/src/main/java/org/opennms/systemreport/system/OSReportPlugin.java Outdated Show resolved Hide resolved
cgorantla
cgorantla approved these changes Jan 31, 2025
View reviewed changes
Copy link
Contributor

@cgorantla cgorantla left a comment

Choose a reason for hiding this comment

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

LGTM, merge latest develop in your branch.

Squash merge when you merge.

@cgorantla
Copy link
Contributor

Only merge when CI/CD pipeline goes green.

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

@naeem-afzal naeem-afzal naeem-afzal left review comments

@Junaid-Khan-Nant Junaid-Khan-Nant Junaid-Khan-Nant left review comments

@cgorantla cgorantla cgorantla approved these changes

@smunir-onms smunir-onms Awaiting requested review from smunir-onms

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mwajahatabbasi @CLAassistant @cgorantla @Junaid-Khan-Nant @naeem-afzal @Shahbaz-dataq