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

[AFF] deserialize INTF metadata as JSON #5704

Merged
merged 34 commits into from Aug 11, 2022
Merged

[AFF] deserialize INTF metadata as JSON #5704

merged 34 commits into from Aug 11, 2022

Conversation

albertmink
Copy link
Member

@albertmink albertmink commented Aug 3, 2022 •
edited

ref to #5658

UT covers the local class only.

Get UT green and then care about refactor (will affect UT for serializations).

@albertmink albertmink added the aff Related to ABAP File Format label Aug 3, 2022
@albertmink albertmink force-pushed the minka/deserialize-aff-metadata branch from 521763a to db04a4c Compare August 3, 2022 20:03
@albertmink albertmink force-pushed the minka/deserialize-aff-metadata branch 2 times, most recently from f2860ae to ba5fe40 Compare August 4, 2022 08:37
@albertmink albertmink force-pushed the minka/deserialize-aff-metadata branch from ba5fe40 to 4bde352 Compare August 4, 2022 08:56
@albertmink
Copy link
Member Author

albertmink commented Aug 4, 2022 •
edited

  • Do UT with JSON metadata
{
  "formatVersion": "1",
  "header": {
    "description": "Example interface for ABAP file formats",
    "originalLanguage": "en"
  }
}

@albertmink albertmink added the G1Y label Aug 4, 2022
@albertmink albertmink force-pushed the minka/deserialize-aff-metadata branch 3 times, most recently from 1c73392 to 8aeaf5a Compare August 4, 2022 15:20
@albertmink albertmink force-pushed the minka/deserialize-aff-metadata branch 2 times, most recently from 52c86b9 to 11caf09 Compare August 4, 2022 16:26
@albertmink albertmink force-pushed the minka/deserialize-aff-metadata branch from 11caf09 to 56ee2e0 Compare August 4, 2022 16:30
@albertmink
Copy link
Member Author

Note: UT are green, however the expected values have to be double checked

albertmink
albertmink commented Aug 5, 2022
View reviewed changes
src/objects/zcl_abapgit_object_intf.clas.locals_imp.abap Outdated

ls_data_aff = iv_data.

ls_data_abapgit-description = set_abapgit_descriptions( is_clsname = 'MYCLASS'
Copy link
Member Author

Choose a reason for hiding this comment

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

Here we need the actual interface name. Otherwise the SAPGUI description texts are not deserialized properly.

@albertmink albertmink removed the G1Y label Aug 5, 2022
@marcushoepfner marcushoepfner marked this pull request as ready for review August 9, 2022 08:35
@marcushoepfner
Copy link
Collaborator

@schneider-michael, @larshp, @crasovan, could you please review

schneidermic0
schneidermic0 requested changes Aug 9, 2022
View reviewed changes
Copy link
Collaborator

@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.

I have added some comments and questions.

src/objects/zcl_abapgit_object_intf.clas.testclasses.abap Show resolved Hide resolved
src/objects/zcl_abapgit_object_intf.clas.locals_imp.abap Outdated Show resolved Hide resolved
src/objects/aff/zcl_abapgit_json_handler.clas.abap Outdated Show resolved Hide resolved
@larshp
Copy link
Member

larshp commented Aug 10, 2022 •
edited

"test-pr / unit-tests" failing: I'm messing with stuff

UPDATE: fixed

crasovan
crasovan reviewed Aug 10, 2022
View reviewed changes
Copy link

@crasovan crasovan 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, new method names are better

@marcushoepfner
Copy link
Collaborator

marcushoepfner commented Aug 10, 2022 •
edited

@larshp could you please review (or ask some other experienced to review)?
I hesitate to click on merge without another review, since it such an essential spot. thanks

schneidermic0
schneidermic0 approved these changes Aug 10, 2022
View reviewed changes
Copy link
Collaborator

@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.

@larshp larshp merged commit 96dd561 into main Aug 11, 2022
@larshp larshp deleted the minka/deserialize-aff-metadata branch August 11, 2022 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larshp larshp larshp left review comments

@marcushoepfner marcushoepfner marcushoepfner left review comments

@crasovan crasovan crasovan left review comments

@schneidermic0 schneidermic0 schneidermic0 approved these changes

Assignees
No one assigned
Labels
aff Related to ABAP File Format
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@albertmink @marcushoepfner @larshp @schneidermic0 @crasovan