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

Upload FUGR and suobjects #11

Merged
merged 25 commits into from
Apr 1, 2021
Merged

Upload FUGR and suobjects #11

merged 25 commits into from
Apr 1, 2021

Conversation

huber-nicolas
Copy link
Contributor

No description provided.

file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/examples/z_aff_example_func.func.json Outdated Show resolved Hide resolved
I updated the Readme and deleted the 2 lines with the empty description arrays of parameters and exceptions in the FUNC.json
file-formats/fugr/format.md Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
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.

Some additions to Christopher's review

file-formats/fugr/examples/z_aff_example_func.func.json Outdated Show resolved Hide resolved
file-formats/fugr/format.md Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/fugr.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
Copy link
Contributor

@albertmink albertmink left a comment

Choose a reason for hiding this comment

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

When merging the PR you might consider to 'squash and merge'. This puts all commits from the PR into a single one which is then commited to the main branch.

@@ -1,6 +1,7 @@
{
"header": {
"$schema": "http://sap.com/schema/reps.json"
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
"$schema": "http://sap.com/schema/reps.json"
"$schema": "http://github.com/SAP/abap-file-formats/file-formats/fugr/reps.json"

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we have to clarify if we refer to blob/master of the file path as suggested here.
I am very uncertain of how this $schema integrates for possible JSON schema validation or JSON editing with text editors. Technically, it must be a URI, nothing more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will discuss it in the #14

removing spaces in uri
formatting the source coding
adding descriptions to func components
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.

Sorry, I found some more

huber-nicolas and others added 3 commits March 23, 2021 16:00
Co-authored-by: Michael Schneider <micha.schneider@sap.com>
Co-authored-by: Michael Schneider <micha.schneider@sap.com>
@albertmink albertmink mentioned this pull request Mar 24, 2021
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.

A small improvement suggestion

file-formats/fugr/func.json Outdated Show resolved Hide resolved
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, Nicolas!

Copy link
Contributor

@Christopher-Hermann Christopher-Hermann left a comment

Choose a reason for hiding this comment

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

Title and description has to be adapted to Title-case/sentence-case. But let's do this within #18

file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/examples/z_aff_example_func.func.json Outdated Show resolved Hide resolved
file-formats/fugr/format.md Show resolved Hide resolved
},
"additionalProperties": false,
"required": [
"includeType"
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the header also a required field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes. We will have to change this in all object types schemas.

@schneidermic0 schneidermic0 self-requested a review March 26, 2021 10:59
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.

Sorry, I have chosen the wrong review status by mistake

Co-authored-by: Michael Schneider <micha.schneider@sap.com>
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, Nicolas :)

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.

Some minor issues

file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
file-formats/fugr/func.json Outdated Show resolved Hide resolved
huber-nicolas and others added 3 commits March 31, 2021 09:16
Co-authored-by: Michael Schneider <micha.schneider@sap.com>
Co-authored-by: Michael Schneider <micha.schneider@sap.com>
Co-authored-by: Michael Schneider <micha.schneider@sap.com>
remove field "host",
change description of field "client"
add field "activeFunctionExit",
remove  following fields from required fields:
"releaseState",
    "global",
    "exceptionClasses",
    "includeNumber",
    "notExecutable"
@huber-nicolas huber-nicolas merged commit efbd0db into main Apr 1, 2021
@albertmink albertmink deleted the Upload-FUGR-and-subobjects branch April 9, 2021 08:56
albertmink added a commit that referenced this pull request Apr 23, 2021
* Upload FUGR and suobjects

* Update Readme and func.json

I updated the Readme and deleted the 2 lines with the empty description arrays of parameters and exceptions in the FUNC.json

* Fix different issues

Fix the issues found by Michael and Christopher

* adapt format.md and fugrtop.reps.json

* replace colon by colon + space

* func: application, client and host are optional

* FUNC: change exception

* Several fixes

removing spaces in uri
formatting the source coding
adding descriptions to func components

* Update file-formats/fugr/examples/lz_aff_example_fugrtop.reps.json

Co-authored-by: Michael Schneider <micha.schneider@sap.com>

* Update file-formats/fugr/examples/saplz_aff_example_fugr.reps.json

Co-authored-by: Michael Schneider <micha.schneider@sap.com>

* Change active to NotExecutable

* change description of "notExecutable"

* func schema group fields

* space after ":"

* Many fixes

* Fix missing "," and space

* Change description of parameter

Co-authored-by: Michael Schneider <micha.schneider@sap.com>

* it is ABAP file formats

* Group rfc fields and change some descriptions

* UXX include not needed

* write global only if true

* change description of "global"

Co-authored-by: Michael Schneider <micha.schneider@sap.com>

* change title and description of "ABAP from Java"

Co-authored-by: Michael Schneider <micha.schneider@sap.com>

* change title and description of "Java from ABAP"

Co-authored-by: Michael Schneider <micha.schneider@sap.com>

* Several changes

remove field "host",
change description of field "client"
add field "activeFunctionExit",
remove  following fields from required fields:
"releaseState",
    "global",
    "exceptionClasses",
    "includeNumber",
    "notExecutable"

Co-authored-by: Michael Schneider <micha.schneider@sap.com>
Co-authored-by: Albert Mink <albert.mink@sap.com>
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.

None yet

4 participants