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

boards/esp32s3_lan9250: configure ethernet universally MAC address #13550

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nuttxs
Copy link
Contributor

@nuttxs nuttxs commented Sep 20, 2024

Summary

boards/esp32s3_lan9250: configure the number of universally administered (by IEEE) MAC addresses.
This PR refers to PR#8993

Impact

The defconfig settings have not been modified.
Provide universal mac addresses support for esp32s3 LAN9250.

Testing

esp32s3-devkit:eth_lan9250

@github-actions github-actions bot added the Size: S The size of the change in this PR is small label Sep 20, 2024
@nuttxpr
Copy link

nuttxpr commented Sep 20, 2024

[Experimental Bot, please feedback here]

The PR summary is missing key information like:

  • Why is this change necessary? What problem does it solve? Is there a bug report or feature request driving this?
  • What functional part of the code is being changed? Be specific - mention files, modules, or functions affected.
  • How does the change work? Provide a more technical explanation of the implementation.

The Impact section is somewhat vague:

  • Is new feature added? Is existing feature changed? Be explicit. This seems like a new feature, state that clearly.
  • Impact on user: "Provide universal mac addresses support" is not descriptive enough. How will users benefit? Will they need to change configurations, use new APIs, etc.?

The Testing section needs significant improvement:

  • Build Host(s): Provide specific details about your development environment (OS version, compiler version, etc.).
  • Testing logs before change / after change: These are placeholders. You need to include actual log snippets demonstrating the issue before the change and the improvement after.

In its current state, the PR would likely not meet NuttX requirements. You need to provide more context, details about the implementation, and clear evidence of testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: S The size of the change in this PR is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants