Skip to content

Rearrange dependencies syntax to be nested in 'requires' object#1573

Merged
carolynvs merged 6 commits intogetporter:v1from
ThorstenHans:feature/dependencies
May 19, 2021
Merged

Rearrange dependencies syntax to be nested in 'requires' object#1573
carolynvs merged 6 commits intogetporter:v1from
ThorstenHans:feature/dependencies

Conversation

@ThorstenHans
Copy link
Contributor

@ThorstenHans ThorstenHans commented May 10, 2021

What does this change

This PR embeds dependencies in an requires object (porter.yaml).

What issue does it fix

Closes #1570

Notes for the reviewer

pkg/porter/install_test.go relies on test data which is pre-populated in the repo. At this point, I was not sure how test data has been generated and if I can replace it. Would be great to chat about that

Checklist

  • Unit Tests
  • Documentation
  • Schema (porter.yaml)

Signed-off-by: Thorsten Hans <thorsten.hans@gmail.com>
Signed-off-by: Thorsten Hans <thorsten.hans@gmail.com>
@carolynvs
Copy link
Member

Just a heads up that I'm "retargeting" your pull request against the brand new v1 branch. We are isolating the changes for v1 in a separate branch to avoid repeatedly breaking things for people.

More details in this PR: #1574

@carolynvs carolynvs changed the base branch from main to v1 May 10, 2021 14:27
Copy link
Member

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

Sweet! This is nearly there. You should do a sweep for any other porter.yaml files that have dependencies in it. That is why the one test is failing but we want to make sure that we find all of them.

Here is the one causing the test to fail: https://github.com/getporter/porter/blob/main/build/testdata/bundles/wordpress/porter.yaml

@carolynvs carolynvs self-assigned this May 10, 2021
Signed-off-by: Thorsten Hans <thorsten.hans@gmail.com>
Signed-off-by: Thorsten Hans <thorsten.hans@gmail.com>
@ThorstenHans
Copy link
Contributor Author

remains the broken test TestPorter_InstallBundle_WithDepsFromTag in pkg/porter/install_test.go

@carolynvs
Copy link
Member

The test is failing because you missed a few porter.yaml files in the repository. Do a find for the word dependencies: and some of them are missing the "requires" parent.

After you update them (some are in documentation, others are test files), the test will pass. Here are a couple that I found but there may be more:

  • build/testdata/bundles/wordpress/porter.yaml
  • docs/content/authors/templates.md

After you get the tests passing, the last change requires is updating the schema file used by porter in the porter schema command to use "requires". See my comment in your other PR for instructions on how to make that change.

Signed-off-by: Thorsten Hans <thorsten.hans@gmail.com>
Signed-off-by: Thorsten Hans <thorsten.hans@gmail.com>
@carolynvs
Copy link
Member

/azp run porter-integration

@azure-pipelines
Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@ThorstenHans ThorstenHans marked this pull request as ready for review May 18, 2021 14:09
@ThorstenHans ThorstenHans requested a review from carolynvs May 19, 2021 06:12
Copy link
Member

@carolynvs carolynvs left a comment

Choose a reason for hiding this comment

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

Thank you for tackling this larger change!

@carolynvs carolynvs merged commit bd14150 into getporter:v1 May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rearrange dependencies syntax in porter.yaml

2 participants