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

Forbid hashed tables in AFF #386

Merged
merged 4 commits into from
Aug 5, 2022
Merged

Forbid hashed tables in AFF #386

merged 4 commits into from
Aug 5, 2022

Conversation

huber-nicolas
Copy link
Contributor

We want to forbid hashed tables, because we do not see any advantage in the AFF. If later there will be use cases, we can remove this limitation.
This is a proposal for issue #176

We want to forbid hashed tables, because we do not see any advantage in the AFF. If later there will be use cases, we can remove this limitation.
@larshp
Copy link
Collaborator

larshp commented Aug 1, 2022

Wondering, perhaps go as far as having everything SORTED TABLE

@@ -65,6 +65,8 @@ Each JSON Schema provided in this repository is automatically generated. For thi

The ABAP types are self-contained, so it is possible to work on them in any system (e.g., in an SAP BTP, ABAP environment system).

Instead of hashed tables, please use sorted tables with a defined key.
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering whether we should add the information to the sub chapter "Type Mapping" below?

There we could also enhance the table. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I enhanced the table. See my newest commit. Please rereview

@schneidermic0
Copy link
Contributor

Wondering, perhaps go as far as having everything SORTED TABLE

Let's start/continue discussion in #176

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.

I suggest just a minor change. Otherwise, it looks good to me.

Co-authored-by: Michael Schneider <[email protected]>
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, @huber-nicolas . LGTM

@huber-nicolas huber-nicolas merged commit c3476eb into main Aug 5, 2022
@huber-nicolas huber-nicolas deleted the Forbid-hashed-tables branch August 5, 2022 08:40
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.

3 participants