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

Keep params below the call #121

Closed
wants to merge 2 commits into from
Closed

Keep params below the call #121

wants to merge 2 commits into from

Conversation

sbcgua
Copy link

@sbcgua sbcgua commented Jan 10, 2020

To #21

Here is my suggestion. :)
If not possible to recommend it this way then at least refer as a grounded alternative and let people choose their preferred way.

P.S. Sorry for long silence with this ...
P.P.S occasionally run a "remove trailing spaces" - will remove these changes if requested not to mix the changes ...

@CLAassistant
Copy link

CLAassistant commented Mar 19, 2020

CLA assistant check
All committers have signed the CLA.

Base automatically changed from master to main March 9, 2021 12:46
@rdibbern rdibbern added the New Rule The issue or PR proposes an new rule or set of rules label Jun 28, 2022
@bjoern-jueliger-sap
Copy link
Member

bjoern-jueliger-sap commented Jul 15, 2022

How is this different from the pattern the guide already mentions as an alternative in If you break, indent the parameters under the call with

DATA(sum) = add_two_numbers(
  value_1 = 5
  value_2 = 6 ).

However, this is the best pattern if you want to avoid the formatting to be broken by a name length change.

Adding an additional section for this seems redundant to me.

@HrFlorianHoffmann
Copy link
Contributor

We discussed this in a decision meeting and came to the following conclusions:

  1. The suggested formatting is sound.
  2. Exactly that formatting is already described in subsequent sections of the guide.
  3. The style of the examples are not in sync with the rest of the guide ("XXXXX" instead of real code).

Especially because of point 2 we think it does not make sense to merge this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clean-abap New Rule The issue or PR proposes an new rule or set of rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants