Skip to content

added HTML, csv example #73

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
Merged
merged 5 commits into from
May 8, 2015
Merged

added HTML, csv example #73

merged 5 commits into from
May 8, 2015

Conversation

pon
Copy link
Contributor

@pon pon commented May 8, 2015

@lob/api-engineering-team -

I created a CSV example that uses HTML, custom fonts, and the data field. You can all use this as the template for adding to your wrapper.

Shoutout @mgartner for having the HTML already done.

@leore
Copy link
Contributor

leore commented May 8, 2015

Sooo sick. LGTM.

I think in the examples section in the readme. It would help to list out the examples we have. Otherwise people will have to click into the folder to figure out what we have. Thoughts? @ami @pon

@pon
Copy link
Contributor Author

pon commented May 8, 2015

Yeah that makes sense

@leore
Copy link
Contributor

leore commented May 8, 2015

Seperate PR?

@elnaz
Copy link
Contributor

elnaz commented May 8, 2015

LGTM!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling d48f128 on csv-example into fc46fd2 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling d48f128 on csv-example into fc46fd2 on master.

@pon
Copy link
Contributor Author

pon commented May 8, 2015

Updated PR with README

leore
leore reviewed May 8, 2015
View reviewed changes
README.md
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about adding the simple ones here as well? @ami

mgartner
mgartner reviewed May 8, 2015
View reviewed changes
examples/csv_postcards/create_postcards.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

this is never used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 13ce9c4 on csv-example into fc46fd2 on master.

robinjoseph08
robinjoseph08 reviewed May 8, 2015
View reviewed changes
examples/csv_postcards/postcard_back.html
Copy link
Contributor

Choose a reason for hiding this comment

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

missing </body>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch - updated

@ami
Copy link
Contributor

ami commented May 8, 2015

@leore the idea was to have a summary in the README but point all the examples to the folder. easier to maintain that way. - what @po has now is good

@ami
Copy link
Contributor

ami commented May 8, 2015

oops @pon lol

robinjoseph08
robinjoseph08 reviewed May 8, 2015
View reviewed changes
examples/README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

why are the titles in this /name/ format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point - I'll remove

@pon
Copy link
Contributor Author

pon commented May 8, 2015

Updated - we good here?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 7a9fa7c on csv-example into fc46fd2 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 7a9fa7c on csv-example into fc46fd2 on master.

@leore
Copy link
Contributor

leore commented May 8, 2015

LGTM

@mgartner
Copy link
Contributor

mgartner commented May 8, 2015

lgtm

pon added a commit that referenced this pull request May 8, 2015
@pon
Merge pull request #73 from lob/csv-example
ad70ca5 
added HTML, csv example
@pon pon merged commit ad70ca5 into master May 8, 2015
@pon pon deleted the csv-example branch May 8, 2015 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@pon @leore @elnaz @coveralls @ami @mgartner @robinjoseph08