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

Avoid libbsd header with libc strlcpy #262

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

yselkowitz
Copy link
Contributor

While #256 avoided the libbsd library dependency, this avoids the header requirement except when libbsd is actually needed.

@stappersg
Copy link
Member

Approving "workflows", to find out what breaks ...

While radvd-project#256 avoided the libbsd library dependency, this avoids the header
requirement except when libbsd is actually needed.

Signed-off-by: Yaakov Selkowitz <[email protected]>
@stappersg
Copy link
Member

I'm not happy how poor the two purposed changes are documented in the commit.

This merge request will be closed.

No hard feelings.

@stappersg stappersg closed this Feb 27, 2025
@yselkowitz
Copy link
Contributor Author

I'm not happy how poor the two purposed changes are documented in the commit.

So far I have proposed two different ways of avoiding the unnecessary inclusion of the libbsd headers, and others are possible, but I'm not psychic. I have yet to hear any feedback on the second, and you could always add to the commit message when merging.

This merge request will be closed.

To what end? Are you not planning on fixing this? Are you planning on creating your own commit to this fix?

No hard feelings.

I find this baffling and counterproductive. Does @robbat2 concur with your decision?

@stappersg
Copy link
Member

stappersg commented Feb 27, 2025 via email

@robbat2
Copy link
Member

robbat2 commented Feb 28, 2025

@stappersg i didn't even have time to review their 2nd version before you closed this. It's supposed to be iterative development; and you don't need "a fresh start" in a constructive environment.
Being able to see the old versions on this PR, after the various force-pushes, and show how it's improved is important.

@robbat2
Copy link
Member

robbat2 commented Feb 28, 2025

Re-opening this so that the CI checking can run on it.

@robbat2 robbat2 reopened this Feb 28, 2025
@robbat2 robbat2 merged commit b9c1ce7 into radvd-project:master Feb 28, 2025
44 checks passed
@stappersg
Copy link
Member

stappersg commented Feb 28, 2025 via email

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

Successfully merging this pull request may close these issues.

3 participants