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

lib: handle visual studio code C/C++ extension #12397

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

rzalamena
Copy link
Member

@rzalamena rzalamena commented Nov 25, 2022

Add some pragmas to handle errors that the C/C++ extension is not able to understand.

See discussion for more details.

@NetDEF-CI
Copy link
Collaborator

NetDEF-CI commented Nov 25, 2022

Continuous Integration Result: SUCCESSFUL

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-8581/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Copy link
Contributor

@eqvinox eqvinox left a comment

Choose a reason for hiding this comment

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

Nits

lib/memory.h Outdated Show resolved Hide resolved
lib/prefix.h Outdated Show resolved Hide resolved
Add some `pragma`s to handle errors that the C/C++ extension is not able
to understand.

Move `TRANSPARENT_UNION` to `lib/compiler.h` for consistency.

Signed-off-by: Rafael Zalamena <[email protected]>
@rzalamena
Copy link
Member Author

@eqvinox thanks! Makes sense. I edited the comments before the pragmas to tell which file to look at for the troubled code since this context was lost with the move.

Should the pragma comment be generic? Or keeping the example/error message is fine? I don't know what is helpful for future readers, but I though the reference could help in the future to decide whether to keep it or not (it could eventually be fixed by visual studio code).

@NetDEF-CI
Copy link
Collaborator

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-8609/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings
Report for compiler.h | 12 issues
===============================================
< ERROR: need consistent spacing around '*' (ctx:WxV)
< #430: FILE: /tmp/f1-1855904/compiler.h:430:
< ERROR: Macros with multiple statements should be enclosed in a do - while loop
< #430: FILE: /tmp/f1-1855904/compiler.h:430:
< WARNING: macros should not use a trailing semicolon
< #430: FILE: /tmp/f1-1855904/compiler.h:430:
< ERROR: Macros with multiple statements should be enclosed in a do - while loop
< #433: FILE: /tmp/f1-1855904/compiler.h:433:
< ERROR: need consistent spacing around '*' (ctx:WxV)
< #434: FILE: /tmp/f1-1855904/compiler.h:434:
< ERROR: need consistent spacing around '*' (ctx:WxV)
< #435: FILE: /tmp/f1-1855904/compiler.h:435:

@rzalamena rzalamena marked this pull request as ready for review November 28, 2022 17:05
@eqvinox
Copy link
Contributor

eqvinox commented Nov 30, 2022

Should the pragma comment be generic? Or keeping the example/error message is fine? I don't know what is helpful for future readers, but I though the reference could help in the future to decide whether to keep it or not (it could eventually be fixed by visual studio code).

I think it's good as it is.

@donaldsharp donaldsharp merged commit 5eb3100 into FRRouting:master Dec 1, 2022
@rzalamena rzalamena deleted the vscode-intro branch December 6, 2022 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants