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

[SPRV] v2 of AFF created and added #619

Closed
wants to merge 15 commits into from

Conversation

delucapietro
Copy link
Contributor

New functionality (custom actions) and a new field to display in the ADT editor where added.
Therefore a new version (v2) for the AFF was created. I added the files next to the existing v1 ones. Maybe the v1 need to be deleted? Please check and let me know.

delucapietro and others added 13 commits June 17, 2024 10:13
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>
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>
@Markus1812
Copy link
Member

Hey @delucapietro
I have noticed you added a non-mandatory field. The documentation states that technically, you don't need a new format version in this case. Are you sure you want to increase the version to v2?

@delucapietro
Copy link
Contributor Author

Hey @Markus1812,

the field itself (implementing class) is filled by coding not directly as an editor or wizard input.
When I understood the general AFF distribution the new field would also be visible in "lower" releases in ADT where the application coding is not released so it would be empty, right?
As the information "no implementing class" is always wrong I decided to update the v2 instead of an empty field.

Copy link
Member

@Markus1812 Markus1812 left a comment

Choose a reason for hiding this comment

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

Some general suggestions. More, if you have answered my question regarding the version upgrade above.

file-formats/sprv/type/zif_aff_sprv_v2.intf.abap Outdated Show resolved Hide resolved
file-formats/sprv/type/zif_aff_sprv_v2.intf.abap Outdated Show resolved Hide resolved
Also updated in coding YI3

Co-authored-by: Markus <Markus1812@users.noreply.github.com>
@Markus1812
Copy link
Member

the field itself (implementing class) is filled by coding not directly as an editor or wizard input.

Is the implementingClass a calculated field and does it need to be transported?

Makes sense, thanks for the suggestion. I also updated the provider name in the coding. Could you suggest the change here aswell so that I can 'commit the suggestion' ?

Co-authored-by: Markus <Markus1812@users.noreply.github.com>
@delucapietro
Copy link
Contributor Author

the field itself (implementing class) is filled by coding not directly as an editor or wizard input.

Is the implementingClass a calculated field and does it need to be transported?

Kind of calculated. The SPRV creates proxy objects. The main proxy object creates an implementing class. Everything is done "automatically" during the creation phase.
As proxy objects are not displayed in ADT it is really easy to find the correct implementing class after activation if the customer is creating multiple SPRVs in the same package e.g. with multiple users at the same time which are named similarly.
So the value is indeed determined but it is referencing a dependent class.

@delucapietro
Copy link
Contributor Author

the field itself (implementing class) is filled by coding not directly as an editor or wizard input.

Is the implementingClass a calculated field and does it need to be transported?

Kind of calculated. The SPRV creates proxy objects. The main proxy object creates an implementing class. Everything is done "automatically" during the creation phase. As proxy objects are not displayed in ADT it is really easy to find the correct implementing class after activation if the customer is creating multiple SPRVs in the same package e.g. with multiple users at the same time which are named similarly. So the value is indeed determined but it is referencing a dependent class.

  • correction: it is not easy to find the implementing class ;)

@Markus1812
Copy link
Member

The SPRV creates proxy objects. The main proxy object creates an implementing class.

Are the proxy object and the implementing class also transported with a SPRV object?

@delucapietro
Copy link
Contributor Author

The SPRV creates proxy objects. The main proxy object creates an implementing class.

Are the proxy object and the implementing class also transported with a SPRV object?

All objects are transported together. The SPRV is always connected to the main proxy object (not bi-directional , the proxy object does not know about the SPRV) and the implementing class is dependent on the proxy object. SPRV is just a wrapper object for the cloud as the proxy objects (SPRX) are not and should not be released for the cloud directly

@Markus1812
Copy link
Member

Thank you for the clarification about the implementationClass field.
From my point of view, the version does not need to be updated to v2, but this is completely up to you. If you decide to up the version to v2, further changes with compatibility are probably needed.

@Markus1812
Copy link
Member

We decided to stay with version 1 as all changes are compatible and do not need a version upgrade. Further changes are proposed in a new Pull Request.

@Markus1812 Markus1812 closed this Jun 19, 2024
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