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

using namespace codal shouldn't be used in header files #240

Closed
carlosperate opened this issue Oct 11, 2022 · 5 comments
Closed

using namespace codal shouldn't be used in header files #240

carlosperate opened this issue Oct 11, 2022 · 5 comments
Assignees
Labels
Milestone

Comments

@carlosperate
Copy link
Contributor

There are a few header files in the different CODAL repositories that have a using namespace codal; line, which forces the namespace into anything that includes it and can cause ambiguity and clashes when integrating with other libraires/SDKs (kind of defeats the point of using a codal namespace to begin with).

I can see maybe a need to have it for MicroBit.h, but if we could remove its usage in all the other header files that would be useful.

@finneyj
Copy link
Contributor

finneyj commented Oct 11, 2022

yes, indeed. I recall this was a necessary evil to get codal-microbit-v2 out before launch. Times were tight!

Correct fix is the explicitly namespace the necessary types in header files to avoid namespace pollution...

@finneyj
Copy link
Contributor

finneyj commented Oct 11, 2022

p.s. I also agree MicroBit.h is a special case, and leaving one in there does make sense I think, in the interesting of keep user programs simpler and not making breaking changes.

@finneyj
Copy link
Contributor

finneyj commented Oct 11, 2022

maybe wrapped in a CONFIG option though, so the user can easily opt out should they wish to.

@carlosperate
Copy link
Contributor Author

maybe wrapped in a CONFIG option though, so the user can easily opt out should they wish to.

Oh yeah, that'd be great!

@microbit-carlos microbit-carlos modified the milestones: v0.2.57, v0.2.60 Jun 15, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.60, v0.2.61 Jul 25, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.62, v0.2.63 Sep 20, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.65, v0.2.66 Nov 3, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.66, v0.2.67 Nov 14, 2023
microbit-carlos pushed a commit to lancaster-university/codal-nrf52 that referenced this issue Jul 22, 2024
As part of:
lancaster-university/codal-microbit-v2#240

Cannot remove all header files usage of `using namesapce` yet as
that will affect other CODAL targets that might not have apply
this type of patch yet.
microbit-carlos pushed a commit to lancaster-university/codal-nrf52 that referenced this issue Jul 22, 2024
As part of:
lancaster-university/codal-microbit-v2#240

Cannot remove all header files usage of `using namesapce` yet as
that will affect other CODAL targets that might not have apply
this type of patch yet.
microbit-carlos pushed a commit to lancaster-university/codal-nrf52 that referenced this issue Jul 22, 2024
As part of:
lancaster-university/codal-microbit-v2#240

Other targets should also apply patches like
bdfa1ec to ensure they don't
depend on the header files having the `using namespace codal`
line.`
carlosperate pushed a commit to carlosperate/codal-core that referenced this issue Jul 23, 2024
And headerfiles declare new clases in the codal namespace.

This is part of:
lancaster-university/codal-microbit-v2#240
microbit-carlos added a commit to lancaster-university/codal-core that referenced this issue Jul 23, 2024
And headerfiles declare new clases in the codal namespace.

This is part of:
lancaster-university/codal-microbit-v2#240

Co-authored-by: Carlos Pereira Atencio <[email protected]>
carlosperate added a commit to carlosperate/codal-nrf52 that referenced this issue Jul 23, 2024
As part of:
lancaster-university/codal-microbit-v2#240

Other targets should also apply patches like
bdfa1ec to ensure they don't
depend on the header files having the `using namespace codal`
line.`
microbit-carlos added a commit to lancaster-university/codal-core that referenced this issue Jul 24, 2024
…al namespace.

This is part of:
lancaster-university/codal-microbit-v2#240

As removing 'using namespace codal' from codal-nrf52 can break
a few targets, this patch adds a CODAL flag to enable/disable
its usage.
microbit-carlos added a commit to lancaster-university/codal-core that referenced this issue Jul 24, 2024
…e. (#171)

Flag mostly to be used in other CODAL libraries/targets.

This is part of:
lancaster-university/codal-microbit-v2#240

As removing 'using namespace codal' from codal-nrf52 can break
a few targets, this patch adds a CODAL flag/macro to enable/disable
its usage.
carlosperate added a commit to carlosperate/codal-nrf52 that referenced this issue Jul 24, 2024
Flag introduced in codal-core in:
lancaster-university/codal-core@509086c

By default the `using namespace` line would still be present, as
removing it can cause some codal targets to break if they depend
on the header file to set up this namespace globally.

For example, the codal-microbit-v2 target had this update to
fix the namespacing:
lancaster-university/codal-microbit-v2#437

This patch is part of:
lancaster-university/codal-microbit-v2#240
carlosperate added a commit to carlosperate/codal-nrf52 that referenced this issue Jul 24, 2024
Flag introduced in codal-core in:
lancaster-university/codal-core@509086c

By default the `using namespace` line would still be present, as
removing it can cause some codal targets to break if they depend
on the header file to set up this namespace globally.

For example, the codal-microbit-v2 target had this update to
fix the namespacing:
lancaster-university/codal-microbit-v2#437

This patch is part of:
lancaster-university/codal-microbit-v2#240
carlosperate added a commit to carlosperate/codal-nrf52 that referenced this issue Jul 24, 2024
Flag introduced in codal-core in:
lancaster-university/codal-core@509086c

By default the `using namespace` line would still be present, as
removing it can cause some codal targets to break if they depend
on the namespace to be set up globally.

For example, the codal-microbit-v2 target had this update to
fix the namespacing:
lancaster-university/codal-microbit-v2#437

This patch is part of:
lancaster-university/codal-microbit-v2#240
microbit-carlos pushed a commit to lancaster-university/codal-nrf52 that referenced this issue Jul 24, 2024
Flag introduced in codal-core in:
lancaster-university/codal-core@509086c

By default the `using namespace` line would still be present, as
removing it can cause some codal targets to break if they depend
on the namespace to be set up globally.

For example, the codal-microbit-v2 target had this update to
fix the namespacing:
lancaster-university/codal-microbit-v2#437

This patch is part of:
lancaster-university/codal-microbit-v2#240
@microbit-carlos microbit-carlos modified the milestones: v0.2.69, v0.2.67 Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants