Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

ConstructorExpressions instead of Macros, No Describe Statement#7

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
sbcgua merged 9 commits intosbcgua:masterfromDerGuteWolf:FixesAndSteampunk
Mar 12, 2021

Conversation

@DerGuteWolf
Copy link
Contributor

Fixes Issue#6
Provides Compatibility with Steampunk
Remove Compativility with < 740

Copy link
Owner

@sbcguasbcgua left a comment

Choose a reason for hiding this comment

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

See some comments below.

Is there a need to make it above 740 ? I'm not using it in older systems but someone can potentially ... so I'm a bit tentative ...

@larshp
Copy link
Contributor

@sbcgua will you be dropping 702 compatibility?

@sbcgua
Copy link
Owner

sbcgua commentedMar 8, 2021
edited
Loading

@larshp I'm pronenot to. But I'm not using this lib currently in my projects in our old system. So ... it depends on who uses it and where. How to find it out ? Maybe we can move 740 support into a branch ?

@DerGuteWolf Could you pls tell what is your reasoning behind this PR ?

@larshp
Copy link
Contributor

larshp commentedMar 8, 2021
edited
Loading

it might be a good example for automatically downporting using abaplint if needed

@sbcgua
Copy link
Owner

it might be a good example for automatically downporting using abaplint if needed

@larshp
I think it is a good case. Let's make a branch for 740 then after this PR is merged. The above changes are not automatable by the way - it is a replacement of macros.

@DerGuteWolf
Copy link
ContributorAuthor

describe is not allowed in steampunk, therefore the reason to remove this. Also Macros are not allowed in steampunk, therefore I substituted macros with VALUE construction expressions

@sbcgua
Copy link
Owner

Thanks !

describe is not allowed in steampunk, therefore the reason to remove this

Would you mind if I ask you to measure the perf before and after and publish here ? If I remember well the test prog is already created - perf_test.prog - though it might need this -https://github.com/sbcgua/benchmarks - I don't remember for sure

@DerGuteWolf
Copy link
ContributorAuthor

Would you mind if I ask you to measure the perf before and after and publish here ?

Will do this, however I need some shuffling around first.

sbcgua reacted with thumbs up emoji

@sbcgua
Copy link
Owner

With describe
image

With RTTI
image

40% slower :( eh ...

@sbcguasbcgua merged commit5a5f84e intosbcgua:masterMar 12, 2021
@sbcgua
Copy link
Owner

sbcgua commentedMar 12, 2021
edited
Loading

OK.

  1. Merged into a separate branch - steampunk
  2. Added a reference to the main readme -https://github.com/sbcgua/abap_mustache/tree/master#versions

@DerGuteWolf Thanks for the PR !
@larshp feel free to PR experiments with auto up-porting. I guess current UTs are able to withstand that. :)

@DerGuteWolfDerGuteWolf deleted the FixesAndSteampunk branchMarch 13, 2021 15:21
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@sbcguasbcguasbcgua 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.

3 participants

@DerGuteWolf@larshp@sbcgua

[8]ページ先頭

©2009-2025 Movatter.jp