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

[#6137] Renamed variables in irods_configuration_keywords.hpp (main) #6332

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

korydraughn
Copy link
Collaborator

@korydraughn korydraughn commented Apr 18, 2022

  • Should KW_ be used instead of KW_CFG_?
  • Do all variables defined in irods_configuration_keywords.hpp reference a config option?

Notice that the data type for these variables has changed from std::string to const char* const. std::string_view would work, but it required adjusting internal APIs and function call invocations. Either way, const char* const provides a lot of flexibility and doesn't break existing APIs.

@alanking
Copy link
Contributor

  • I would be fine with either. I think keeping the CFG_ prefix in some instances would be good, but that would require more careful consideration as is being suggested in question 2.
  • No. For instance, not all of the plugin types are used as configuration keywords, as near as I can tell.

Please confirm that our plugins and shipped clients still work with these changes. Otherwise, seems fine to me. Would like an opinion from @trel and/or @SwooshyCueb as well.

@trel
Copy link
Member

trel commented Apr 18, 2022

No strong opinions here.

Not as familiar with most of these. Current changes appear to just move _KW to the front, yes? And const.

@korydraughn
Copy link
Collaborator Author

Cool. I'll leave this PR as is and plan to handle the plugins.

And yes, I think the use of KW_CFG_ for configuration variables is fine. The placement of KW_ is the real change here.

Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

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

I think this is good to go, barring any other comments

@korydraughn
Copy link
Collaborator Author

Passed core tests.

@korydraughn
Copy link
Collaborator Author

Will add the #'s soon

@korydraughn korydraughn changed the title [#6137] Adjusted variable naming conventions used by irods_configuration_keywords.hpp (main) [#6137] Renamed variables in irods_configuration_keywords.hpp (main) Apr 26, 2022
@korydraughn
Copy link
Collaborator Author

Added #

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants