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.

Sign up for GitHub

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

Jump to bottom

Make zif_aff_log and others public #19

Merged
merged 17 commits into from
Mar 23, 2022
Merged

Make zif_aff_log and others public #19

merged 17 commits into from
Mar 23, 2022

Conversation

wurzka
Copy link
Contributor

@wurzka wurzka commented Mar 22, 2022 •
edited
Loading

Closes #18

wurzka and others added 8 commits March 22, 2022 08:15
@wurzka
Make zif_aff_log %26 others public
05aad80
@wurzka
Reaction to abaplint
e4edad2
@wurzka
More abaplint fixes
766fa23
@wurzka @abaplint
Update src/zcl_aff_generator.clas.testclasses.abap
7fa9606 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@wurzka @abaplint
Update src/zcl_aff_generator.clas.testclasses.abap
72e77ff 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@wurzka @abaplint
Update src/zcl_aff_log.clas.abap
47302d0 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@wurzka
Keywords to upper
2680504
@wurzka
Reaction to abaplint
55cc7de
@larshp
Copy link
Collaborator

larshp commented Mar 22, 2022

two of the issues fixed with open-abap/open-abap-core#293

wurzka and others added 2 commits March 22, 2022 08:43
@wurzka
abaplint
f936f3c
@wurzka @abaplint
Update src/zcl_aff_writer_xslt.clas.testclasses.abap
2f378e7 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
This was linked to issues Mar 22, 2022
Rename saff_core to zaff_tools #20
Closed
Replace cx_aff_root by zcx_aff_tools #21
Closed
larshp
larshp approved these changes Mar 22, 2022
View reviewed changes
Copy link
Collaborator

@larshp larshp left a comment

Choose a reason for hiding this comment

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

lgtm, suggest merging, main is also not green

@albertmink albertmink self-requested a review March 22, 2022 13:29
albertmink
albertmink approved these changes Mar 22, 2022
View reviewed changes
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.

LGTM, this is an iterative effort. So keeping the PRs small is a good choice.

schneidermic0
schneidermic0 requested changes Mar 22, 2022
View reviewed changes
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.

See my feedback, below

Katharina want me to blog merging this ;)

src/zcl_aff_tools_unit_test_helper.clas.abap Show resolved Hide resolved
src/zcl_aff_log.clas.testclasses.abap Outdated Show resolved Hide resolved
src/z_aff_tools.msag.xml Outdated Show resolved Hide resolved
src/zcx_aff_tools.clas.abap Show resolved Hide resolved
schneidermic0 and others added 6 commits March 22, 2022 14:34
@schneidermic0
back to standard rule for local_class_naming
d54bd5a
@wurzka
Rename message class
de95e06
@wurzka @abaplint
Update src/zcl_aff_generator.clas.testclasses.abap
e8f0d99 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@wurzka @abaplint
Update src/zcl_aff_generator.clas.testclasses.abap
4dddc6b 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@wurzka @abaplint
Update src/zcl_aff_writer_xslt.clas.testclasses.abap
45c6dca 
Co-authored-by: abaplint[bot] <24845621+abaplint[bot]@users.noreply.github.com>
@wurzka
Remove local exception class
cfb86bd
schneidermic0
schneidermic0 approved these changes Mar 23, 2022
View reviewed changes
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.

LGTM

@wurzka wurzka merged commit c8d6844 into main Mar 23, 2022
@wurzka wurzka deleted the wurzka/log branch March 23, 2022 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@larshp larshp larshp approved these changes

@albertmink albertmink albertmink approved these changes

@schneidermic0 schneidermic0 schneidermic0 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@wurzka @larshp @albertmink @schneidermic0