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

adding "extern C" guard #1144

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Conversation

CheMax-Tag
Copy link

@CheMax-Tag CheMax-Tag commented Mar 11, 2021

I add missing "extern C" guard for some header files.

This can help to stub out the error described in #617 when using the C++ compiler.

I plan to check all the headers and add missing guards if you don't mind.

@JonatanAntoni
Copy link
Member

Thanks @CheMax-Tag, may I ask you to update all file versions and dates of files you've touched, please?
I consider this change (it is non-functional) as a patch level one, hence the file versions should be incremented in the third diget.

Cheers,
Jonatan

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Please change file versions (increase 3rd digit) and dates in the file headers.

@CheMax-Tag
Copy link
Author

Ok. I'm sorry, this is essentially the first request in my practice).

Another question, the date indicated in the copyright should also be updated?
I mean the year, for example, in this lines:

// Copyright (c) 2017-2020 IAR Systems //Copyright (c) 2017-2019 Arm Limited. All rights reserved.

should I change the year to 2021?

CheMax added 2 commits March 11, 2021 11:31
moving the guard higher, since he fell under the definition.
@ghost
Copy link

ghost commented Mar 11, 2021

Yes, please change the year to 2021.

@CheMax-Tag
Copy link
Author

Ok, it's ready.

******************************************************************************/
/*
* Copyright (c) 2020 Arm Limited. All rights reserved.
* Copyright (c) 2021 Arm Limited. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

Copyright should state "2020-2021" not just 2021

******************************************************************************/
/*
* Copyright (c) 2020 Arm Limited. All rights reserved.
* Copyright (c) 2021 Arm Limited. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

Please change to "2020-2021"

@JonatanAntoni
Copy link
Member

Thanks for taking this over and contributing. May I ask you to sqash all your commits into a single one, please?

@CheMax-Tag
Copy link
Author

Thanks for taking this over and contributing. May I ask you to sqash all your commits into a single one, please?

Yes, it will take me a while

@CheMax-Tag
Copy link
Author

I'm a little confused with branches collapsing commits.
Please understand, forgive.

Correct commit with all changes 38236c1

@JonatanAntoni
Copy link
Member

Hi @CheMax-Tag,

no worries. The PR cannot be merged this way. It looks like you introduced a merge commit.
You need to rebase (!) your changes onto the latest develop branch. During rebase you can squash all commits into a single one.

Cheers,
Jonatan

@JonatanAntoni
Copy link
Member

Can one of the admins verify this patch?

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.

2 participants