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

Pull request for newly created object type SPRV #369

Merged
merged 11 commits into from
Jun 28, 2022
Merged

Pull request for newly created object type SPRV #369

merged 11 commits into from
Jun 28, 2022

Conversation

delucapietro
Copy link
Contributor

It seems that no example was created by report SAFF_GENERATE_REPO.

@delucapietro
Add files via upload
caa5cd3 
It seems that no example was created by report SAFF_GENERATE_REPO
@cla-assistant
Copy link

cla-assistant bot commented Jun 27, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

schneidermic0
schneidermic0 reviewed Jun 27, 2022
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 @delucapietro for your contribution. I have added some (first) questions

sprv/type/zif_aff_sprv_v1.intf.abap Outdated Show resolved Hide resolved
sprv/type/zif_aff_sprv_v1.intf.abap Outdated Show resolved Hide resolved
sprv/type/zif_aff_sprv_v1.intf.abap Outdated Show resolved Hide resolved
delucapietro and others added 2 commits June 27, 2022 15:12
@delucapietro
Update regarding the feedback
c7583ee
@delucapietro @abaplint
Update file-formats/sprv/type/zif_aff_sprv_v1.intf.abap
d36ab33 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@schneidermic0
Copy link
Contributor

It seems that no example was created by report SAFF_GENERATE_REPO.

I'll check with the colleagues what might be wrong here

delucapietro and others added 4 commits June 27, 2022 15:28
@delucapietro
abaplint formatting fix
588bcce
@schneidermic0 @abaplint
Update file-formats/sprv/type/zif_aff_sprv_v1.intf.abap
758fabb 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@schneidermic0 @abaplint
Update file-formats/sprv/type/zif_aff_sprv_v1.intf.abap
b629603 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@schneidermic0 @abaplint
Update file-formats/sprv/type/zif_aff_sprv_v1.intf.abap
73c1f82 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@huber-nicolas
Copy link
Contributor

@delucapietro
The report does not create the example object for you. You have to create it on your own manually beforehand.
The report then only serializes the example object (into its json AFF format) and adds it to the github folder.

@delucapietro
Copy link
Contributor Author

@delucapietro The report does not create the example object for you. You have to create it on your own manually beforehand. The report then only serializes the example object (into its json AFF format) and adds it to the github folder.

@huber-nicolas
Is there any Guide Line on how to do this? Since it is a completely new object there is no entry in the creation option. I just can add an entry to the header table without the generated subobjects.

schneidermic0
schneidermic0 reviewed Jun 28, 2022
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 the missing example.

@delucapietro @huber-nicolas
Update file-formats/sprv/README.md
add0c2b 
Sure, thank you for checking

Co-authored-by: Nicolas Huber <[email protected]>
wurzka
wurzka requested changes Jun 28, 2022
View reviewed changes
Copy link
Contributor

@wurzka wurzka left a comment

Choose a reason for hiding this comment

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

Just one title case issue

file-formats/sprv/type/zif_aff_sprv_v1.intf.abap Outdated Show resolved Hide resolved
@huber-nicolas huber-nicolas mentioned this pull request Jun 28, 2022
Open
@huber-nicolas
Copy link
Contributor

I made an issue as a reminder to upload the example object later. See issue #370
So we can merge this pull request and upload the example later

@delucapietro @wurzka
Update file-formats/sprv/type/zif_aff_sprv_v1.intf.abap
e208a3b 
Thanks for checking, I will go with 'Header data' - without 'data' I feel like something is missing

Co-authored-by: Katharina Wurz <[email protected]>
@albertmink albertmink assigned albertmink and unassigned albertmink Jun 28, 2022
@albertmink
Copy link
Contributor

Let me know when you want to merge. The Check Markdown links is not related to the pull request, so don't bother with that.

@huber-nicolas
Copy link
Contributor

Let me know when you want to merge. The Check Markdown links is not related to the pull request, so don't bother with that.

You can merge now

@albertmink albertmink merged commit 6ef6aeb into SAP:main Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schneidermic0 schneidermic0 schneidermic0 left review comments

@huber-nicolas huber-nicolas huber-nicolas approved these changes

@wurzka wurzka wurzka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@delucapietro @schneidermic0 @huber-nicolas @albertmink @wurzka