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

Remove obsolete parameter enum typedefs #137

Closed
msmk0 opened this issue Apr 24, 2020 · 4 comments · Fixed by #420
Closed

Remove obsolete parameter enum typedefs #137

msmk0 opened this issue Apr 24, 2020 · 4 comments · Fixed by #420
Assignees
Labels
Component - Core Affects the Core module Impact - Major Significant bug and/or affects a lot of modules Improvement Changes to an existing feature
Milestone

Comments

@msmk0
Copy link
Contributor

msmk0 commented Apr 24, 2020

Replace the usage of the following deprecated enum typefs

using ParDef = BoundParametersIndices;
using ParID_t = BoundParametersIndices;
using ParValue_t = BoundParametersScalar;

in favor of the specific underlying enums and remove them from the ParameterDefinition.hpp file.

This is a larger change that needs to be split over multiple PRs.

@msmk0 msmk0 added Component - Core Affects the Core module Improvement Changes to an existing feature Impact - Major Significant bug and/or affects a lot of modules labels Apr 24, 2020
@acts-issue-bot acts-issue-bot bot removed the Triage label Apr 24, 2020
@paulgessinger
Copy link
Member

The other names are sooooo much longer... :/

@msmk0
Copy link
Contributor Author

msmk0 commented Apr 27, 2020

That was kind-of the point of introducing them; to have descriptive names that allow more than one parameters type.

It does not really matter that much for the indices enums. Since they must be regular enums (as discussed in the context of their introduction the enum values do not need be qualified. You can use use e.g. eBoundPhi instead of BoundParametersIndices::eBoundPhi.

Depending on the decision in #151 we might consider shortening them a bit, e.g. to just

enum BoundIndices { ... };
using BoundScalar = ...;

However, that must happen before we start removing the obsoleted typedefs.

@stale
Copy link

stale bot commented Jun 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale label Jun 11, 2020
@stale stale bot closed this as completed Jun 25, 2020
@paulgessinger paulgessinger reopened this Jun 26, 2020
@stale stale bot removed the Stale label Jun 26, 2020
@stale
Copy link

stale bot commented Jul 26, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Stale label Jul 26, 2020
@stale stale bot closed this as completed Aug 9, 2020
@msmk0 msmk0 reopened this Aug 10, 2020
@stale stale bot removed the Stale label Aug 10, 2020
@msmk0 msmk0 added this to the v1.00.00 milestone Aug 27, 2020
@msmk0 msmk0 self-assigned this Sep 1, 2020
@msmk0 msmk0 closed this as completed in #420 Sep 2, 2020
msmk0 added a commit that referenced this issue Sep 2, 2020
This performs the following renames
```
BoundParametersIndices -> BoundIndices
BoundParametersScalar -> BoundScalar
eBoundParametersSize -> eBoundSize
FreeParametersIndices -> FreeIndices
FreeParametersScalar -> FreeScalar
eFreeParametersSize -> eFreeSize
```
and replaces the following previously deprecated typedefs with the correct types
```
ParDef -> BoundIndices
ParID_t -> BoundIndices
ParValue_t -> BoundScalar
```

Fixes #137 and fixes #407.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Core Affects the Core module Impact - Major Significant bug and/or affects a lot of modules Improvement Changes to an existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants