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

Make skip_download a boolean instead of string #37

Merged
merged 2 commits into from
Sep 24, 2018

Conversation

itsdalmo
Copy link
Contributor

@itsdalmo
Copy link
Contributor Author

I'm a bit confused about why boolean strings vs proper booleans are used interchangeably in other resources, when both source and params seem to support bool just fine. E.g., looking at other resources written in Go:

In the S3 resource:

  • skip_download is a string, and they use strconv.ParseBool to convert it to a boolean.
  • skip_download is also a parameter in the source configuration... but here it is a boolean.
  • And there seems to be no difference between params and source configurations in terms of handling booleans, since the get params also accept a boolean in unpack.

In the concourse pipeline resource, it seems to be the other way around:

But why? 🤷‍♂️

@mikael-lindstrom
Copy link

Maybe we should ask/create an issue with concourse to figure this out? Seems very strange.

@itsdalmo
Copy link
Contributor Author

Vito responded on Discord that true booleans should be fine, no need to use a string and convert it👍

Copy link

@mikael-lindstrom mikael-lindstrom left a comment

Choose a reason for hiding this comment

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

Great, then this LGTM 👍

@itsdalmo itsdalmo merged commit e61d418 into master Sep 24, 2018
@itsdalmo itsdalmo deleted the make-skip-download-boolean branch September 24, 2018 07:39
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.

2 participants