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

Add more SFP/interface statistics & ability to print #196

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

byteit101
Copy link
Contributor

This PR does 3 things:

  1. It adds more SFP/SFP+ statistics
  2. Allows configuration of whether unplugged/link-lost sfp modules are still logged
  3. Adds a new print target for the above sfp/interface details

Currently tested and working on my CRS305

@akpw
Copy link
Owner

akpw commented Oct 19, 2024

@byteit101, thanks for this.
A few quick comments:

  • the the naming, given the new option is specific to SFP would it make sense to change it something like monitor_sfp_unplugged?
  • regarding the licensing, pls check out the discussion here and feel free to adjust the license or remove related code from the commit.

@byteit101
Copy link
Contributor Author

I have updated this PR and solve the conflicts

@akpw
Copy link
Owner

akpw commented Oct 27, 2024

thanks. the cli output formatter still seems out of sync with the project license and as mentioned above, this project is governed by that one. If that is an issue of any kind, maybe simply remove the file in question from the PR?

@byteit101
Copy link
Contributor Author

All of my contributions are under GPLv2+ as the license states, so I am not entirely sure what you mean by "out of sync". The byline is obviously going to differ if individual authors are named, which is why in several other projects that I am a contributor to, they use the "project contributors" or similar language to avoid an ever-growing list of authors

@akpw
Copy link
Owner

akpw commented Oct 28, 2024

'Out of sync' means the licenses in that new file is not, in the license terms, the one used to govern this project. As you yourself pointed out, it’s also not intended as a place to list individual contributions, nor is it meant to differentiate people's contributions based on addition of an arbitrary new file.

@byteit101
Copy link
Contributor Author

Perhaps there is some translation issue or something, but the licenses are the same, it's the copyright byline that differs. Either way, fix it up however you want

@akpw
Copy link
Owner

akpw commented Nov 15, 2024

the copyright notice there is a part of the license, and should be in sync before merge

@byteit101
Copy link
Contributor Author

Is this a difference in EU vs US copyright law that I'm not aware of? I have not heard such an assertion before. Either way, I have synchronized all the bylines.

@akpw
Copy link
Owner

akpw commented Nov 25, 2024

thanks, though still unsure why to update existing license in all the project files?

as per original request, could you pls just put the current project license into the only new file in your commit, or just remove that file for now not to block the rest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants