Skip to content

Conversation

@Akshit6828
Copy link

@Akshit6828 Akshit6828 commented May 4, 2021

Updated glb_director_config.h according to the naming convention of the C language standard.

The following changes mentioned below are proposed in order to uphold the reserve identifier violation of the C language standard:

  • identifier '_GLB_DIRECTOR_CONFIG_H' changed to GLB_DIRECTOR_CONFIG_H
  • Indentifier '__IPT_GLBREDIRECT_MATCH__' changed to IPT_GLBREDIRECT_MATCH

…IPT_GLBREDIRECT_MATCH__' according to the naming conventions of C language.
Copy link

@elfring elfring left a comment

Choose a reason for hiding this comment

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

  • Please avoid a typo in the commit subject.
  • Would it be helpful to explain the change another bit in the commit description?
  • Are there any further update candidates to consider?

@elfring
Copy link

elfring commented May 5, 2021

How do you think about to improve the wording also a bit for the pull request subject?

@Akshit6828
Copy link
Author

Sure @elfring. I'll update the changes and let you know shortly! Thanks for mentioning the changes precisely.

@Akshit6828 Akshit6828 changed the title FIX #31 : Renamed identifiers according to named convention of identifiers of C Language. Modified identifiers according to the named convention of identifiers of C Language standards. May 5, 2021
@Akshit6828 Akshit6828 changed the title Modified identifiers according to the named convention of identifiers of C Language standards. Modified identifiers according to the named convention of identifiers of C language standards. May 5, 2021
@elfring
Copy link

elfring commented May 5, 2021

Wording “… naming convention of the C language standard”?

@Akshit6828 Akshit6828 changed the title Modified identifiers according to the named convention of identifiers of C language standards. Modified identifiers according to the naming convention of the C language standard. May 5, 2021
@Akshit6828
Copy link
Author

The commit 438eedf propose changes as explained below to uphold the reserve identifier violation of C language standards

Change 1

Changed pdnet.h with the following changes :

  • Identifier _PDNET_H_ is changed to PDNET_H

Change 2

Changed packet_parsing.h with the following changes :

  • Identifier _PACKET_PARSING_H_ is changed to PACKET_PARSING_H

@Akshit6828
Copy link
Author

@elfring please review the commit messages and proposed changes and let me know if any alterations are needed.

@elfring
Copy link

elfring commented May 5, 2021

@Akshit6828
Copy link
Author

Ok, @elfring. I'll note this feedback and propose changes as described by you as early as possible.

@elfring
Copy link

elfring commented May 5, 2021

Is your interest growing for any source code search patterns which I mentioned for some software components?

@Akshit6828
Copy link
Author

Akshit6828 commented May 5, 2021

@elfring sorry to say, but I have less idea regarding how to extend the search of source code as this was my first contribution to any big company's project and that's why I am not aware of practices to extend the search for the source code.

But I am interested in learning new things. I am also reading the resource provided by you.
Please correct me if I am not getting your directions.

@elfring
Copy link

elfring commented May 5, 2021

Do you get any further development ideas from linked information sources?

@Akshit6828
Copy link
Author

Actually @elfring I am a little confused as changes proposed by me in commits changed the identifiers which were reserved words.
As this resource says that

"All identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag namespaces."

I think that in my proposed changes, the identifiers should begin with an underscore, and the rest of the changes are alright.

Am I correct sir?

@Akshit6828
Copy link
Author

Actually @elfring I am a little confused as changes proposed by me in commits changed the identifiers which were reserved words.
As this resource says that

"All identifiers that begin with an underscore are always reserved for use as identifiers with file scope in both the ordinary and tag namespaces."

I think that in my proposed changes, the identifiers should begin with an underscore, and the rest of the changes are alright.

Am I correct sir?

Like:

  • GLB_DIRECTOR_CONFIG_H -->_GLB_DIRECTOR_CONFIG_H
  • IPT_GLBREDIRECT_MATCH --> _IPT_GLBREDIRECT_MATCH
  • PDNET_H ---> _PDNET_H
  • PACKET_PARSING_H --> _PACKET_PARSING_H

@elfring
Copy link

elfring commented May 5, 2021

Please take another look at the rules for the identifier selection. - Your search approach seems to be incomplete so far.

@Akshit6828
Copy link
Author

Akshit6828 commented May 5, 2021

ok you mean, I need to search more files and identifiers for such issues?

@elfring
Copy link

elfring commented May 5, 2021

I propose to increase source code analysis accordingly.
Did you notice where I reported similar open issues?

@Akshit6828
Copy link
Author

I've gone through the issues and I couldn't see similar issues.
It means for increasing source code analysis accordingly, I need to check where the same issue exists.

Sir, please correct me if I am wrong in understanding you.

or

If you don't mind can you point my work precisely that what changes I should do so that I can understand my work better?

@elfring
Copy link

elfring commented May 5, 2021

… and I couldn't see similar issues.

Do you find the following search approaches interesting?

@Akshit6828
Copy link
Author

Ok sir I'll check these search approaches and change the source code accordingly within few days.
Thanks for mentioning them.

@elfring
Copy link

elfring commented May 7, 2021

🤔 Would you like to check any source code places besides questionable include guards?

@Akshit6828
Copy link
Author

🤔 Would you like to check any source code places besides questionable include guards?

Hi @elfring. Sorry, I didn't get that. Could you elaborate your question?

@elfring
Copy link

elfring commented May 8, 2021

@Akshit6828
Copy link
Author

No, I am not familiar with it. But I'll read the resource and let you know.

@github-service-catalog
Copy link

It looks like this PR might be missing Durable Ownership Scorecard check. Here is what to do:

  • 🟠 Expected — Waiting for workflow to run (stuck) - Your PR probably predates the repository enrollment.
    • Please reopen the PR or push a new commit to trigger the check.
  • ✅ Successful in XXs — Workflow passed - Your PR has a passing check:
    • Great! No further action is needed - this message can be ignored or deleted.

You can read more about this check on TheHub. If you have any questions, please reach out in #service-catalog image

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