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

Added mailgun example. #19

Merged
merged 1 commit into from
Oct 23, 2015
Merged

Added mailgun example. #19

merged 1 commit into from
Oct 23, 2015

Conversation

jmdobry
Copy link
Member

@jmdobry jmdobry commented Oct 21, 2015

PORT: 8080
```

Create a `config.js` in the root of your application with the following

Choose a reason for hiding this comment

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

All of the other GAE node examples will use environment variables. Can you use that for consistency or do you feel strongly about config.js?

Copy link
Member Author

Choose a reason for hiding this comment

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

How would the environment variable be set? I think it's unsafe (and not a practice I want to encourage) to put keys/secrets into a file that gets committed to source control. config.js is ignored by git. The Geddy sample also uses a git-ignored file:

Choose a reason for hiding this comment

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

I'm really torn on this. We need to be consistent with the other samples, which all use environment variables.

Copy link
Member Author

Choose a reason for hiding this comment

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

One of my major friction points on my initial project was: How do I provide config to my app in various environments? I couldn't set environment variables without committing config to my app's source control (not gonna do that), so I used a git-ignored config file.

If we want the little samples to be simpler by just committing config to source control, then for the samples that have config I think there should be a disclaimer that says something like "In a production app, here's how you might get config into your app: <link to article about it or link to built-for-production sample app, e.g. LabelCat>"

@theacodes
Copy link

In general, this samples seems to have too much code that isn't related to what it's demonstrating. The part about sending mail is fine though.

@theacodes
Copy link

After our offline discussion, do you want to update this PR or start a new one with the more bare-bones sample?

@jmdobry
Copy link
Member Author

jmdobry commented Oct 23, 2015

I'm updating this PR right now

@theacodes
Copy link

👍

@jmdobry
Copy link
Member Author

jmdobry commented Oct 23, 2015

Updated

1. Add a [new domain](https://mailgun.com/app/domains).
1. Find your API key in your new domain's settings.

### Create a new Node.js app

Choose a reason for hiding this comment

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

This section title doesn't match the content. Maybe remove this section and move its content up to the first section "Mailgun on Google App Engine".

@jmdobry
Copy link
Member Author

jmdobry commented Oct 23, 2015

Changes made

> [Mailgun](https://www.mailgun.com/): The Email Service For Developers

This example uses [Express.js](http://expressjs.com) and
[node-mailgun](http://github.com/shz/node-mailgun).

Choose a reason for hiding this comment

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

One more nit:

This sample application demonstrates how to use [Express.js](http://expressjs.com) and [node-mailgun](http://github.com/shz/node-mailgun) to send transactional email on [Google App Engine](https://cloud.google.com/appengine).

jmdobry added a commit that referenced this pull request Oct 23, 2015
@jmdobry jmdobry merged commit fadbcae into master Oct 23, 2015
@jmdobry jmdobry deleted the mailgun branch October 23, 2015 17:35
telpirion pushed a commit that referenced this pull request Nov 9, 2022
🤖 I have created a release \*beep\* \*boop\*
---
## [1.1.0](https://www.github.com/googleapis/nodejs-contact-center-insights/compare/v1.0.2...v1.1.0) (2021-07-30)


### Features

* update CCAI Insights protos. change nesting of Conversation.Transcript.Participant to ConversationParticipant and remove AnnotationBoundary.time_offset ([#19](https://www.github.com/googleapis/nodejs-contact-center-insights/issues/19)) ([7501a27](https://www.github.com/googleapis/nodejs-contact-center-insights/commit/7501a276f8159fb7fdc21aad1a19d108fde61c8a))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
ace-n pushed a commit that referenced this pull request Nov 11, 2022
ace-n pushed a commit that referenced this pull request Nov 11, 2022
ace-n pushed a commit that referenced this pull request Nov 14, 2022
grayside pushed a commit that referenced this pull request Nov 15, 2022
ace-n pushed a commit that referenced this pull request Nov 15, 2022
ace-n pushed a commit that referenced this pull request Nov 15, 2022
grayside pushed a commit that referenced this pull request Nov 16, 2022
* Upgrade repo-tools and regenerate scaffolding.

* fix lint task in circleci
ace-n pushed a commit that referenced this pull request Nov 17, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
ace-n pushed a commit that referenced this pull request Nov 17, 2022
* Upgrade repo-tools and regenerate scaffolding.

* fix lint task in circleci
grayside pushed a commit that referenced this pull request Nov 17, 2022
* Upgrade repo-tools and regenerate scaffolding.

* fix lint task in circleci
ahrarmonsur pushed a commit that referenced this pull request Nov 17, 2022
* Upgrade repo-tools and regenerate scaffolding.

* fix lint task in circleci
NimJay pushed a commit that referenced this pull request Nov 19, 2022
* chore: lock files maintenance

* chore: lock files maintenance
NimJay pushed a commit that referenced this pull request Nov 19, 2022
* chore: lock files maintenance

* chore: lock files maintenance
jsimonweb pushed a commit to jsimonweb/nodejs-docs-samples that referenced this pull request Dec 12, 2022
kweinmeister pushed a commit that referenced this pull request Jan 11, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
telpirion pushed a commit that referenced this pull request Jan 11, 2023
* add try/catch to delete operations in testing

* refactor test file; use timestamp for resource names, clean up resources over an hour old, move create and delete operations to before and after hooks in each test suite

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
irataxy added a commit that referenced this pull request Jan 13, 2023
* add try/catch to delete operations in testing

* refactor test file; use timestamp for resource names, clean up resources over an hour old, move create and delete operations to before and after hooks in each test suite

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
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