Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Update github.com/mjibson/esc #515

Merged
merged 3 commits into from
Jan 29, 2020
Merged

Update github.com/mjibson/esc #515

merged 3 commits into from
Jan 29, 2020

Conversation

TomOnTime
Copy link
Collaborator

  • Update github.com/mjibson/esc
  • Fix generate.go to new signature for esc.Run

@tlimoncelli
Update github.com/mjibson/esc
e1671e0 
* Update github.com/mjibson/esc
* Fix generate.go to new signature for esc.Run
@TomOnTime
Copy link
Collaborator Author

Question for @mjibson: I noticed the calling signature for esc.Run changed. Now all old code needs a bunch of template code to create the io.Writer. Wouldn't it make sense to detect a nil writer and create the file automatically?

@TomOnTime TomOnTime mentioned this pull request Jul 1, 2019
Open
@maddyblue
Copy link

Yes, that would make sense. Most people (I believe) use the CLI and not the API, so moving this logic didn't seem to matter, but yes it should just do that.

@tlimoncelli tlimoncelli merged commit 92b51db into master Jan 29, 2020
@tlimoncelli tlimoncelli deleted the update-emed branch January 30, 2020 15:11
rblenkinsopp pushed a commit to rblenkinsopp/dnscontrol that referenced this pull request Aug 21, 2020
@TomOnTime
Update github.com/mjibson/esc (StackExchange#515)
6c15e78 
* Update github.com/mjibson/esc

* Update github.com/mjibson/esc
* Fix generate.go to new signature for esc.Run

* Internal: Upgrade to lastest "esc"
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TomOnTime @maddyblue @tlimoncelli