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

Add additional IBDO parameter diff options. #609

Merged

Conversation

jdcove2
Copy link
Collaborator

@jdcove2 jdcove2 commented Oct 5, 2023

Currently, IBaseDataObjectDiffHelper returns all of the parameter keys and values for both IBDO's if there is any difference. This can make finding the actually differences difficult if there are a lot of parameters.

This PR does the following:

  1. Add an option to display only the keys that differ for both IBDO's.
  2. Add an option to display only the keys and values that differ for both IBDO's.
  3. Make No. 1 the default behavior instead of the current behavior.

@jpdahlke jpdahlke added this to the v8.0.0-M8 milestone Oct 5, 2023
@jpdahlke jpdahlke added the enhancement An enhancement or update to an existing feature label Oct 6, 2023
@drivenflywheel
Copy link
Collaborator

drivenflywheel commented Oct 10, 2023

3. Make No. 1 the default behavior instead of the current behavior.

We're replacing the default behavior with a new behavior; is there a way to explicitly get the current behavior or is the current behavior being abandoned?

EDIT: Ah, we're making "1." the default behavior, but it's an implicit-only "option". The way you specify it is to not specify an incompatible option.

@jdcove2
Copy link
Collaborator Author

jdcove2 commented Oct 10, 2023

Sorry ... poorly phrased. This PR will make:

  1. Default behavior be "different keys only"
  2. Optional behavior, KEY_VALUE_PARAMETER_DIFF, is "different keys and values only"
  3. Optional behavior, DETAILED_PARAMETER_DIFF, is the current behavior of "all keys and values".

Copy link
Collaborator

@drivenflywheel drivenflywheel left a comment

Choose a reason for hiding this comment

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

Minor tweaks suggested

@jdcove2 jdcove2 force-pushed the AddIbdoDiffParameterOptions branch 2 times, most recently from b295e01 to 084ddcf Compare October 10, 2023 20:40
drivenflywheel
drivenflywheel previously approved these changes Oct 11, 2023
@jpdahlke jpdahlke modified the milestones: v8.0.0-M8, v8.0.0-M9 Oct 11, 2023
@arp-0984 arp-0984 self-requested a review October 11, 2023 15:54
Copy link
Collaborator

@arp-0984 arp-0984 left a comment

Choose a reason for hiding this comment

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

Only minor comments, overall looks good though.

@cfkoehler cfkoehler merged commit a35b765 into NationalSecurityAgency:master Nov 7, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or update to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants