Renaming an existing parameter names leads to inconsistencies

[buca92]

Hello colleagues,

after meging the PR #5329, all out object, that consume your repository, lead to syntax inconsistency. It is caused by renaming exporting parameter names from eo_... to ei_....

At first, it does not make sense at all. The exporting parameter always assign object (that impoements given interface), not interface itself so I see no reason why it should be named in this way.

At second, this causes syntax inconsistencies in all objects that consume this.

At third, this causes inconsistencies cross AbapGit versions.

Please, roll the parameters back to previous state to avoid this. Interface of released/published objects cannot be changed because they are consumed in many places.

Many thanks.

[larshp]

However, they do not provide a guaranteed API. Future changes are a possibility.

https://docs.abapgit.org/development/api.html

Because of this, all changes/pull requests to abapGit are checked for incompatible changes before merge, and reported under "abaplint / cross check", example = https://github.com/abapGit/abapGit/pull/5464/checks?check_run_id=5954458688

For https://github.com/abapGit/abapGit/pull/5329 we did discover all incompatible changes and fixed those along with the merge, see comment https://github.com/abapGit/abapGit/pull/5329#issuecomment-1045966072

Can you add a link to the repository that was broken?

[buca92]

The code is, unfortunatelly, placed in our internal github and we cannot share this. I can only put here a piece of code that became inconsistent

[larshp]

we will keep improving and refactoring abapGit.

abapGit APIs are not stable as such, keeping it stable will cause the development to slow down, and make it more difficult to add features.

two possibilities, as I see it,

a: open source the code, if possible, then the syntax errors will be proactively found and fixed b: adjust the internal processes to discover and fix incompatible changes

[buca92]

Or simply stop using the Bulgarian notation. Variable prefixes are not needed in any popular languages so why it should be needed in ABAP.

[larshp]

🎪

that is the example problem, but not the generic problem, repo might be renamed to repository or changed to a RETURNING parameter also causing inconsistencies

[buca92]

sure, of course, that is right.

[jfilak]

that is the example problem, but not the generic problem, repo might be renamed to repository or changed to a RETURNING parameter also causing inconsistencies

Correct, so do not do that! These cosmetic changes are contraproductive. Think twice before you name a variable. Be a tough reviewer and do not let a sub-optimal code in - but once you let it in, live with it. That's what I have learnt during my 5 years era of being Linux package maintainer.

[fabianlupa]

Think twice before you name a variable. Be a tough reviewer and do not let a sub-optimal code in - but once you let it in, live with it.

In general that is a good approach, but I do want to add this: In the very very very small ABAP open source scene you want to encourage people to join in and help out (in their free time nonetheless...) and not scare them away. We already have lots of barriers like needing an ABAP system in the first place, then not using any of the language features or APIs statically released in like 15 years to be 702 compatible and also not using any features that would be incompatible with the copy-paste one report approach, for example by learning and using the internal ui framework.

But we also have many (custom made...) tools in place at this point that help with that like running abaplint for style, syntax and cross reference check (and unit tests) as well as abapmerge to check the full build on pull requests. Also for many use cases you don't need to know ancient browser specific versions of html/css/js that are only compatible with the SAP GUI Html Viewer anymore as there is a nice abstraction layer available.

So I think we already do quite a lot to help keep a balance between encouraging pull requests but also enforcing guidelines systematically and checking for quality manually in reviews.

Sure you can do even more but at some point it's not about tooling or guidelines but about maintenance and release strategy and philosophy, move fast and break and fix things or never change a running system.

Is this now a "package" for external reuse? If so that might require a different approach in a layer on top in another repo or just a fork (though I thought it was already forked with proprietary code added but maybe that's a different use case).

Edit: just to be clear, of course there should have been a note about the change in question in the change log and it should be updated more often.

[pokrakam]

This is a classic case against Hungarian prefix notation: Changing a variable's type means having to change its name. Here it is a perfectly sensible change to swap a class to an interface, which would remain unchanged in a prefixless code base.

But I understand abapGit started long ago and is now >100k LoC of consistent prefixing (thanks to abaplint) that can't just be changed.

[sbcgua]

Imho, we should be more specific with the context. Does abapGit have public API ? which is supposed to be stable? afaik, the answer is no. Maybe we should look into the direction of an "official" API layer.

• Which would be based on interfaces only (thus would easy the naming prefixes problem, maybe it should even we without prefixes ...).
• Which would be small (to be able to guarantee it's stability).
• And which would be more respected in term of frequent changes

Then it could be compared to the "Linux package" case.

The API can be by the way both ABAP and REST (which seems to be very relevant after that gCTS thing had been presented)

[fabianlupa]

For reference as not everyone might have followed the huge slack thread, there's wip over at https://github.com/abapGit/abapgit-api-rfc

[sbcgua]

Heh, thanks, I was too lazy to read 150+ msgs ... let me check it :)

[mbtools]

stale. closing