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.

Sign up for GitHub

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

Jump to bottom

Update ENHS Type #129

Merged
merged 16 commits into from
Jul 22, 2021
Merged

Update ENHS Type #129

merged 16 commits into from
Jul 22, 2021

Conversation

wurzka
Copy link
Contributor

@wurzka wurzka commented Jul 20, 2021 •
edited
Loading

closes #117

@wurzka wurzka requested a review from schneidermic0 July 20, 2021 06:17
@wurzka wurzka self-assigned this Jul 20, 2021
@wurzka wurzka requested a review from marcushoepfner July 20, 2021 07:01
marcushoepfner
marcushoepfner reviewed Jul 20, 2021
View reviewed changes
Copy link

@marcushoepfner marcushoepfner left a comment

Choose a reason for hiding this comment

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

in general there are also other so-called enhancement technologies than badi_def.
that's why there is a tool_type mentioned in the enhancement spot. but I guess we can add more later to the schema (I think we don't need others yet) and have a new "type" next to badi_definitions in ty_main then.

file-formats/enhs/enhs.json Show resolved Hide resolved
file-formats/enhs/enhs.json Outdated Show resolved Hide resolved
schneidermic0
schneidermic0 requested changes Jul 20, 2021
View reviewed changes
Copy link
Contributor

@schneidermic0 schneidermic0 left a comment

Choose a reason for hiding this comment

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

See my comments.

As soon as we have adapted the format I suggest to reintroduce an example which was deleted (currently) by this PR

file-formats/enhs/type/zif_seef_aff_enhs_v1.intf.abap Outdated Show resolved Hide resolved
file-formats/enhs/type/zif_seef_aff_enhs_v1.intf.abap Show resolved Hide resolved
file-formats/enhs/type/zif_seef_aff_enhs_v1.intf.abap Show resolved Hide resolved
file-formats/enhs/type/zif_seef_aff_enhs_v1.intf.abap Outdated Show resolved Hide resolved
schneidermic0
schneidermic0 requested changes Jul 20, 2021
View reviewed changes
Copy link
Contributor

@schneidermic0 schneidermic0 left a comment

Choose a reason for hiding this comment

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

Looks good to me, except for the removed example. Do you plan to add id?

Just see my additional comment. You can decide whether, you want to change it.

@wurzka
Copy link
Contributor Author

wurzka commented Jul 20, 2021

The example will be added as soon as I get the 'ok' for the type for ENHS.

schneidermic0
schneidermic0 approved these changes Jul 20, 2021
View reviewed changes
Copy link
Contributor

@schneidermic0 schneidermic0 left a comment

Choose a reason for hiding this comment

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

Here you go :)

@wurzka wurzka requested a review from schneidermic0 July 21, 2021 13:40
schneidermic0
schneidermic0 reviewed Jul 21, 2021
View reviewed changes
Copy link
Contributor

@schneidermic0 schneidermic0 left a comment

Choose a reason for hiding this comment

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

See my comment

file-formats/enhs/type/zif_seef_aff_enhs_v1.intf.abap Outdated
@@ -173,9 +173,6 @@ INTERFACE zif_seef_aff_enhs_v1
"! <p class="shorttext">Example Classes of the BAdI Definition</p>
"! Example classes of the BAdI definition
example_classes TYPE ty_example_classes,
"! <p class="shorttext">Use Fallback Class</p>
"! Use fallback class in case no implementation is executed
use_fallback_class TYPE abap_bool,
"! <p class="shorttext">Name of the Default/Fallback Class</p>
"! Name of the default/fallback class
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add the meaning of the fallback class in the description?

Suggested change
"! Name of the default/fallback class
"! Name of the default/fallback class. The fallback class is executed if no BAdI implementation exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I like it

schneidermic0
schneidermic0 approved these changes Jul 21, 2021
View reviewed changes
Copy link
Contributor

@schneidermic0 schneidermic0 left a comment

Choose a reason for hiding this comment

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

Thanks. looks good to me

@wurzka wurzka merged commit ab25ce5 into main Jul 22, 2021
@wurzka wurzka deleted the feature/enhs_v1 branch July 22, 2021 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schneidermic0 schneidermic0 schneidermic0 approved these changes

@marcushoepfner marcushoepfner marcushoepfner approved these changes

Assignees

@wurzka wurzka

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No ABAP type for ENHS, yet
3 participants
@wurzka @marcushoepfner @schneidermic0