Skip to content

Conversation

antoniovazquezblanco
Copy link
Contributor

No description provided.

@antoniovazquezblanco antoniovazquezblanco marked this pull request as draft June 4, 2025 18:37
@antoniovazquezblanco antoniovazquezblanco force-pushed the libserialport branch 3 times, most recently from 5941b3a to 83aaa78 Compare June 4, 2025 19:02
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file should be generated with configure_file(), not hardcoded in the repo. Not every define in the upstream config.h.in is actually used, but some are and we should perform the appropriate feature tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @bgilbert for the pointers. Currently looking into this. Will try to update soon.

Thanks! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now compiling. Thanks for the pointers! :D

Copy link
Collaborator

Choose a reason for hiding this comment

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

The feature checks are still missing, and they generally look as though they'd be straightforward to implement. I think it'd be better to match upstream's checks as much as possible (especially since upstream doesn't have functional tests) unless you have a good argument for why they aren't necessary.

@antoniovazquezblanco antoniovazquezblanco force-pushed the libserialport branch 7 times, most recently from 9515936 to fad548d Compare June 13, 2025 18:19
@antoniovazquezblanco antoniovazquezblanco marked this pull request as ready for review June 13, 2025 18:26
@antoniovazquezblanco antoniovazquezblanco marked this pull request as draft July 16, 2025 21:24
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