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

fix(bind): interpret bind defined as type []map[string]string #302

Merged
merged 1 commit into from
Dec 14, 2022
Merged

fix(bind): interpret bind defined as type []map[string]string #302

merged 1 commit into from
Dec 14, 2022

Conversation

darshilgit
Copy link
Contributor

@darshilgit darshilgit commented Jun 15, 2022 •
edited
Loading

As per the type definition of Layer, bind is of type Bind which is a
struct with fields Source and Dest.

But when we do UnmarshalYAML for type Bind, we are assuming that we
would always get the Bind as a []string and not
[]map[interface{}]interface{} (when making a call to
getStringOrStringSlice).

This PR fixes the getStringOrStringSlice to consider Bind defined
as a []map[string]string and returns appropriate bind string
in that case.

Fixes #301

@mikemccracken
Copy link
Contributor

mikemccracken commented Jun 15, 2022 via email

@darshilgit darshilgit force-pushed the fix-issue-301 branch 2 times, most recently from 1cc875e to cb6819d Compare June 16, 2022 17:48
@mikemccracken mikemccracken self-requested a review June 16, 2022 19:35
mikemccracken
mikemccracken requested changes Jun 16, 2022
View reviewed changes
Copy link
Contributor

@mikemccracken mikemccracken left a comment

Choose a reason for hiding this comment

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

Since we're just adding a simple syntax test that the struct syntax for binds works, how about moving this to a new file test/binds.bats, and add a simplified test for the old syntax too?

@darshilgit darshilgit requested a review from mikemccracken June 16, 2022 20:23
@darshilgit darshilgit force-pushed the fix-issue-301 branch 2 times, most recently from 31b8580 to 67162dc Compare June 16, 2022 20:26
@hallyn
Copy link
Contributor

hallyn commented Sep 26, 2022

@darshilgit can you rebase and repush to retrigger build? (the build logs have been purged)

@hallyn hallyn changed the base branch from master to main November 21, 2022 14:55
@rchincha
Copy link
Contributor

rchincha commented Dec 14, 2022 •
edited
Loading

I might need this in PR #349

hallyn
hallyn approved these changes Dec 14, 2022
View reviewed changes
Copy link
Contributor

@hallyn hallyn 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

@darshilgit darshilgit force-pushed the fix-issue-301 branch 5 times, most recently from 0adcb57 to 113b5c4 Compare December 14, 2022 18:32
@darshilgit darshilgit changed the title interpret Bind as well when getting string or slice from interface{} fix(bind): interpret bind defined as type []map[string]string Dec 14, 2022
shimish2
shimish2 reviewed Dec 14, 2022
View reviewed changes
@darshilgit darshilgit force-pushed the fix-issue-301 branch 2 times, most recently from ae7505d to bb741c6 Compare December 14, 2022 18:49
@darshilgit darshilgit requested review from shimish2 and hallyn and removed request for smoser, rchincha, shimish2 and hallyn December 14, 2022 18:52
@rchincha
Copy link
Contributor

Not particularly happy with that we are turning golang to loosey-goosey javascript here, but a cleanup for another day.

@darshilgit darshilgit force-pushed the fix-issue-301 branch 3 times, most recently from 8750faf to 6caa2f7 Compare December 14, 2022 19:20
@darshilgit
fix(bind): interpret bind defined as type []map[string]string
8d0ab4f 
As per the type definition of Layer, bind is of type Bind which is a
struct with fields Source and Dest.

But when we do UnmarshalYAML for type Bind, we are assuming that we
would always get the Bind as a []string and not
[]map[interface{}]interface{} (when making a call to
getStringOrStringSlice).

This PR fixes the getStringOrStringSlice to consider Bind defined
as a []map[string]string and returns appropriate bind string
in that case.

Fixes project-stacker#301

Signed-off-by: Darshil Parikh <darshil.parikh@gmail.com>
@rchincha rchincha merged commit 73a37c9 into project-stacker:main Dec 14, 2022
@rchincha rchincha added this to the v0.40.1 milestone Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rchamarthy rchamarthy rchamarthy approved these changes

@hallyn hallyn hallyn approved these changes

@mikemccracken mikemccracken mikemccracken approved these changes

@shimish2 shimish2 shimish2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.40.1
Development

Successfully merging this pull request may close these issues.

error while unmarshalling stacker file (binds field)
6 participants
@darshilgit @mikemccracken @hallyn @rchincha @rchamarthy @shimish2