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.

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

⚠️ Add "owner" and "contributors" columns to application import CSV format. #551

Merged
merged 2 commits into from
Nov 8, 2023

Conversation

mansam
Copy link
Collaborator

@mansam mansam commented Nov 7, 2023

Adds two fields to the import CSV immediately before tags. One is for the Owner and should be empty or contain a single value in the form 'John Doe [email protected]'. The second is for contributors, and should be empty or a list of one or more values in the previous format, separated by commas. As this would be a comma-delimited field within a CSV, the whole value should be surrounded by quotes. This breaks compatibility with any existing import CSVs since it adds columns in the middle (necessary since the number of tags is unbounded and as such the tag list must come last).

Fixes #538

@jortel Do you think we can get away with adding this to migration 11, or do you think I should create a new one?

Adds two fields to the import CSV immediately before tags.
One is for the Owner and should be empty or contain a single
value in the form 'John Doe <[email protected]>'. The second
is for contributors, and should be empty or a list of one or
more values in the previous format, separated by commas. As
this would be a comma-delimited field within a CSV, the whole
value should be surrounded by quotes.

Fixes konveyor#538

Signed-off-by: Sam Lucidi <[email protected]>
@mansam mansam requested review from aufi and jortel November 7, 2023 23:10
@mansam mansam changed the title Enable importing app stakeholders ⚠️ Enable importing app stakeholders Nov 7, 2023
@mansam mansam changed the title ⚠️ Enable importing app stakeholders ⚠️ Add "owner" and "contributors" columns to application import CSV format. Nov 7, 2023
Copy link
Member

@aufi aufi left a comment

Choose a reason for hiding this comment

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

@mansam mansam merged commit 57bb3f0 into konveyor:main Nov 8, 2023
10 of 11 checks passed
@aufi
Copy link
Member

aufi commented Nov 8, 2023

@mansam I think Stakeholders created by CSV import test might need to be cleaned-up after the CSV import test (to not fail other tests).

@fabianvf
Copy link
Contributor

fabianvf commented Nov 8, 2023

Why did this merge with failing CI?

For future reference, we should merge the API test updates and the PR that breaks CI simultaneously. This change broke the integration CI across all repos. We can make changes to the API tests repo (https://github.com/konveyor/go-konveyor-tests) and ensure it works in this branch by adding

        Go tests PR: <PR number>

to a PR description, which will make it run against the proposed changes to ensure the CI stays green across the org as we merge these changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSV template for import doesn't contain columns for Owner and Contributors
3 participants