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

Attempt to implement template debugging tool #6114

Merged
merged 39 commits into from
Mar 13, 2019
Merged

Attempt to implement template debugging tool #6114

merged 39 commits into from
Mar 13, 2019

Conversation

qubitrenegade
Copy link
Contributor

@qubitrenegade qubitrenegade commented Feb 3, 2019 •
edited
Loading

This pull request is related to #5065 and is currently in progress.

Please refer to #5065 for discussion.

@thesentinels
Copy link
Contributor

Thanks for the pull request! Here is what will happen next:

  1. Your PR will be reviewed by the maintainers
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

@qubitrenegade qubitrenegade mentioned this pull request Feb 3, 2019
Closed
@qubitrenegade qubitrenegade force-pushed the qbr/add-plan-render-command branch 14 times, most recently from 24c9650 to 72d9ce5 Compare February 3, 2019 12:03
@baumanj
Copy link
Contributor

baumanj commented Feb 4, 2019

Thanks so much for working on this! I'm probably not going to have bandwidth to give a proper review for a couple days at least, but I'll definitely take a look when I can.

@christophermaier
Copy link
Contributor

This is also in my queue to review... looking forward to taking a look!

@baumanj
Copy link
Contributor

baumanj commented Feb 7, 2019

We haven't forgotten about you, but since I'm out tomorrow, I just wanted to let you know that Monday is the earliest I'll be able to start reviewing this. Thanks for your patience!

@christophermaier
Copy link
Contributor

I'm in the middle of reviewing this now, FYI.

@qubitrenegade
no_render_dir to no_render
aeece66 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
rustfmt fueled cleanup
ec8cb3a
@qubitrenegade
use @baumanj method of merging json structs.
a4886f1 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
rebase master. add back the unwrap for now... does #TODO: fix using '…
9a5f35f 
….unwrap()' work?

Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
Add inspec file for consul_config.json
a159501 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
return Result<Json> instead of Json
bff2b63
@qubitrenegade
First pass at adding Error type for serde_json::Error
c960a88 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
Remove unwrap() and .expect(); propagate errors. Add broken test toml…
ff0e44f 
…/json files.

Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
re-revert changes to support/linux/install_dev_9_linux.sh
60339a3 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
apply shellcheck-and-rustfmt-fixes.patch.txt
6508e13 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
Fix use of no_render_dir
f7e7be7 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
simplify parsing *_path variables
3b08980 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
!(quiet) to !quiet
e7c43c5 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
{:?} to {}
e1f94c9 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
simplify getting file_name
874bee5 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
add broken template to test/fixtues and test/fixtures/render/error di…
cbaa742 
…rectory to shellcheck exclude
@qubitrenegade
rustfmt cleanup
0f3c654 
Signed-off-by: qubitrenagade <[email protected]>
@qubitrenegade
Copy link
Contributor Author

Alright, I think that's everything.

I think renaming NO_WRITE_FILE to NO_RENDER, then when parsing it let render = !m.is_present("NO_RENDER"); is the most elegant solution? re: not no render.

@qubitrenegade
NO_WRITE_FILE to NO_RENDER
e3e7666 
Signed-off-by: qubitrenagade <[email protected]>
@baumanj baumanj merged commit 259c934 into habitat-sh:master Mar 13, 2019
chef-ci added a commit that referenced this pull request Mar 13, 2019
@chef-ci
Update CHANGELOG.md with details from pull request #6114
8cac6f3 
Obvious fix; these changes are the result of automation not creative thinking.
@qubitrenegade qubitrenegade deleted the qbr/add-plan-render-command branch March 14, 2019 04:12
@qubitrenegade
Copy link
Contributor Author

qubitrenegade commented Mar 14, 2019 •
edited
Loading

w00t! And added to the change log!

This is awesome 🎉

Thanks for all the help along the way too! I learned a ton! (as to how much I retain...)

Github is dumb sometimes, was trying to respond inline but doesn't seem to be an option anymore...

re unwrap() convo: I think that makes sense. unwrap() is an "unsafe" operation, so we try to avoid it where we can because the compiler can't ensure safety (hence the panic); we can reason about how our code is unreachable so an unwrap() is safe because there's no way an "unsafe" value could be passed?

Really, thanks for all the back and forth. It really has been educational. (and an overall great experience! :) )

This is so cool! 😀

@baumanj baumanj mentioned this pull request Jun 14, 2019
Closed
@christophermaier christophermaier added Type:Feature PRs that add a new feature and removed X-feature labels Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christophermaier christophermaier christophermaier left review comments

@baumanj baumanj baumanj approved these changes

@chefsalim chefsalim Awaiting requested review from chefsalim

@eeyun eeyun Awaiting requested review from eeyun

@fnichol fnichol Awaiting requested review from fnichol

@jamesc jamesc Awaiting requested review from jamesc

@raskchanky raskchanky Awaiting requested review from raskchanky

@reset reset Awaiting requested review from reset

Assignees

@christophermaier christophermaier

@baumanj baumanj

Labels
Type:Feature PRs that add a new feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@qubitrenegade @thesentinels @baumanj @christophermaier