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

Changes to commonPipelineEnvironment for ABAP #2035

Merged
merged 2 commits into from
Sep 17, 2020
Merged

Changes to commonPipelineEnvironment for ABAP #2035

merged 2 commits into from
Sep 17, 2020

Conversation

DanielMieg
Copy link
Member

@DanielMieg DanielMieg commented Sep 16, 2020 •
edited
Loading

Changes

  • Tests
  • Documentation

It was originally planned to pass repositoryNames through this pipeline. repositoryNames is superseded by the more extensive addonDescriptor.

The resourceRef abap/repositories also relates to an earlier design and was never fully implemented. This is not required anymore.

@DanielMieg DanielMieg requested a review from CCFenner September 16, 2020 14:51
@DanielMieg
Copy link
Member Author

/it

@DanielMieg DanielMieg mentioned this pull request Sep 16, 2020
Merged
2 tasks
@DanielMieg
Copy link
Member Author

Removing obsolete coding and preparing the commonPipelineEnvironment for #2023

@DanielMieg
Copy link
Member Author

/it

stippi2
stippi2 reviewed Sep 17, 2020
View reviewed changes
Copy link
Member

@stippi2 stippi2 left a comment

Choose a reason for hiding this comment

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

Can you please give a description for this PR? "Changes to..." is not helpful, I see that you changed something, but it makes no sense to me "as is". In the YAML, you removed the reference to the CPE resource, without replacing it. I don't understand how it is still supposed to work.

@DanielMieg
Copy link
Member Author

Can you please give a description for this PR? "Changes to..." is not helpful, I see that you changed something, but it makes no sense to me "as is". In the YAML, you removed the reference to the CPE resource, without replacing it. I don't understand how it is still supposed to work.

Added a description. "is still supposed to work" -> I think it never worked

@stippi2
Copy link
Member

stippi2 commented Sep 17, 2020

Added a description. "is still supposed to work" -> I think it never worked

Thanks. So this is just a preparation for when ABAP steps read this resource under its new name, correct?

@DanielMieg
Copy link
Member Author

Added a description. "is still supposed to work" -> I think it never worked

Thanks. So this is just a preparation for when ABAP steps read this resource under its new name, correct?

yes

@DanielMieg DanielMieg merged commit 836075e into SAP:master Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stippi2 stippi2 stippi2 approved these changes

@CCFenner CCFenner Awaiting requested review from CCFenner

@OliverNocon OliverNocon Awaiting requested review from OliverNocon

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DanielMieg @stippi2