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

[SCP1] Added Fields, Removed Fields, Updated Fields, Title, Comment and Annotations added #676

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

Rits-SAP
Copy link

@Rits-SAP Rits-SAP commented Dec 3, 2024

Software Component, Minimum Release and Maximum Release removed- These are not any more relevant to BC Sets.
Constants not relevant currently for the BC Set creation are removed.

Hard coded Constants replaced with our Interface Constants.
"Field Attribute" changed from field_attribute_value to field_attribute.

Scope Relevant added- This is a new feature in our BC Sets.
Added relevant annotations wherever required.
Position added- To denote the position of the sub-BC Sets in case of hierarchical BC Sets

Title and comment added or modified wherever required.

Copy link

cla-assistant bot commented Dec 3, 2024

CLA assistant check
All committers have signed the CLA.

@huber-nicolas
Copy link
Contributor

I have one more question. Did you already deliver SCP1 to an on-premise release or is SCP1 used in abapGit or used in XCO Api? Because then we have to check whether the changes you made are compatible or not.
See also https://github.com/SAP/abap-file-formats/blob/main/docs/json.md#incompatible-file-format-changes

@Rits-SAP
Copy link
Author

Rits-SAP commented Dec 9, 2024

I have one more question. Did you already deliver SCP1 to an on-premise release or is SCP1 used in abapGit or used in XCO Api? Because then we have to check whether the changes you made are compatible or not. See also https://github.com/SAP/abap-file-formats/blob/main/docs/json.md#incompatible-file-format-changes

Regarding the above question:
SCP1 is used in abapGit - https://docs.abapgit.org/user-guide/reference/supported.html
You can check the above link.

Regarding XCO, integrations is not fully complete, but you can check the package S_XCO_GENERATION_SCP1

If you have further questions we can connect

@huber-nicolas
Copy link
Contributor

I have one more question. Did you already deliver SCP1 to an on-premise release or is SCP1 used in abapGit or used in XCO Api? Because then we have to check whether the changes you made are compatible or not. See also https://github.com/SAP/abap-file-formats/blob/main/docs/json.md#incompatible-file-format-changes

Regarding the above question: SCP1 is used in abapGit - https://docs.abapgit.org/user-guide/reference/supported.html You can check the above link.

Regarding XCO, integrations is not fully complete, but you can check the package S_XCO_GENERATION_SCP1

If you have further questions we can connect

Together with my colleagues I analysed the situation and we checked the abapGit handler of SCP1. Since the abapGit for SCP1 is not using this Abap File Format of SCP1 we can do these changes to the AFF of SCP1 without increasing the format version.

@huber-nicolas
Copy link
Contributor

I see that you have updated the if_aff_scp1_v1 in the abap system and implemented the changes which I recommended. Can you please also update this pull request so that we see the changes in github. Thanks

@Rits-SAP
Copy link
Author

I have a query, currently we are not able to open any BC Set in YI3, is it like it will work only after the pull request is approved.

@huber-nicolas
Copy link
Contributor

I have a query, currently we are not able to open any BC Set in YI3, is it like it will work only after the pull request is approved.

No the pull request approve/merge does not have any effect on the abap backend.
I have just checked in YI3, for me the SCP1 editor is working fine. Can you please recheck from you side

@Rits-SAP
Copy link
Author

The interface should be created in the same package you want to use for the development objects for Workbench Integration.
Currently our interface and workbench integration are in different packages, what will be the consequence?

@huber-nicolas
Copy link
Contributor

The interface should be created in the same package you want to use for the development objects for Workbench Integration. Currently our interface and workbench integration are in different packages, what will be the consequence?

From our side it is not mandatory that the interface is in the same package as the other objects.

name TYPE c LENGTH 32,
"! <p class="shorttext">Position</p>
"! Position of BC Set in Hierarchical BC Set
"! $maximum 10
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"! $maximum 10
"! $minimum 1
"! $maximum 10

I just saw in the schema that for integers we write "minimum": -2147483648 into the schema. This does not make sense for the field "position_in_hierarchy". Can you please add a "minimum" annotation to overrule the value of -2147483648.

Copy link
Author

Choose a reason for hiding this comment

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

added the $minimum annotation and updated the Files.

@huber-nicolas huber-nicolas added the ux-review ready AFF is ready for UX review label Dec 18, 2024
@huber-nicolas
Copy link
Contributor

For me from AFF perspective it is ok. I just want to involve the UX colleagues to check whether the changes are also ok from UX perspective, since your AFF changes also had impact on the editor UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ux-review ready AFF is ready for UX review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants