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

/ui2/cl_json: add inherit functionality #831

Merged
merged 24 commits into from
Jan 20, 2024
Merged

Conversation

oblomov-dev
Copy link
Contributor

open-abap is not working when a class inherits /ui2/json:
https://github.com/abap2UI5/abap2UI5/actions/runs/7583976799/job/20656717865?pr=793

added:

  • constructor
  • is_compressable
  • serialize_int (and added the code of the serialize method)
  • added attributes used in the constructor

next step / issue:

  • adjusting serialize_int with respecting the value mv_extended.

I have no idea if this inheriting concept is easily implementable in open-abap? before i start making changes here without knowing how this all specifically works, maybe its better you comment on this first.

oblomov-dev and others added 14 commits June 29, 2023 09:44
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@larshp larshp changed the title add inherit functionality /ui2/cl_json: add inherit functionality Jan 19, 2024
@larshp larshp changed the title /ui2/cl_json: add inherit functionality /ui2/cl_json: add inherit functionality Jan 19, 2024
@larshp
Copy link
Member

larshp commented Jan 19, 2024

inheritance should work in open-abap, so all good

oblomov-dev and others added 4 commits January 19, 2024 14:29
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@larshp
Copy link
Member

larshp commented Jan 19, 2024

@oblomov-dev if you dont have vscode installed, you can try https://github.com/github/dev, it might be easier

@oblomov-dev
Copy link
Contributor Author

yes github codespaces (i think it is the same) is my second home after eclipse :) i will check it!

@oblomov-dev
Copy link
Contributor Author

ok, fixed all the linter issues now, all tests are green.

i skipped implementing the call of the "is_compressable" method, means from my understanding: you can inherit /ui2/cl_json now but when you create a unit test based on a change implemented in is_compressable, open-abap will give a different result than the normal abap stack but maybe for the first step, this is enough.

Copy link
Member

@larshp larshp left a comment

Choose a reason for hiding this comment

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

see comments

src/json/#ui2#cl_json.clas.abap Outdated Show resolved Hide resolved
src/json/#ui2#cl_json.clas.abap Show resolved Hide resolved
src/json/#ui2#cl_json.clas.abap Outdated Show resolved Hide resolved
src/json/#ui2#cl_json.clas.abap Outdated Show resolved Hide resolved
src/json/#ui2#cl_json.clas.abap Outdated Show resolved Hide resolved
@oblomov-dev
Copy link
Contributor Author

see this review and comment function for the first time, nice! code is adjusted.

@oblomov-dev oblomov-dev requested a review from larshp January 20, 2024 08:21
Copy link
Member

@larshp larshp left a comment

Choose a reason for hiding this comment

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

thanks 👍

@larshp
Copy link
Member

larshp commented Jan 20, 2024

if the compress inheritance and stuff doesnt work, suggest as next step adding a unit test for it in https://github.com/oblomov-dev/open-abap-core/blob/main/src/json/%23ui2%23cl_json.clas.testclasses.abap

@larshp larshp merged commit 5f599f6 into open-abap:main Jan 20, 2024
6 checks passed
@oblomov-dev
Copy link
Contributor Author

@larshp
ok, would there also be the chance to debug the code? can i for example rename it to zui2_cl_json, copy it to an abap stack and run the unit tests or what is the recommended way?

@larshp
Copy link
Member

larshp commented Jan 20, 2024

that might work

typically I dont debug much, when there is a simple unit test for it its easy to find the bug

standalone debuggin also works a bit,
image

@oblomov-dev
Copy link
Contributor Author

ah ok interesting, i will try it out :)

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

Successfully merging this pull request may close these issues.

2 participants