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

Minor documentation updates #35

Merged
merged 6 commits into from
May 17, 2018
Merged

Minor documentation updates #35

merged 6 commits into from
May 17, 2018

Conversation

vparpoil
Copy link
Contributor

@vparpoil vparpoil commented May 9, 2018

  • Generation of settings in the UI for new apps
  • MAIL_URL env variable
  • Sorting of the apps by name

Copy link
Owner

@lmachens lmachens left a comment

Choose a reason for hiding this comment

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

Thank you for your pr!
I added some comments

### Setup steps
### Setup steps using MUP

Keep in mind when using MUP: [your user account on the server must be sudoer without password](http://meteor-up.com/docs.html#ssh-based-authentication-with-sudo)
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this necessary? It works for me without.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the links give the details about this. maybe your username is root ?

Copy link
Owner

Choose a reason for hiding this comment

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

no, but doesn't matter. this comment won't hurt :)

@@ -20,7 +20,8 @@ module.exports = {
env: {
ROOT_URL: 'https://apm.YOUR_DOMAIN.com',
MONGO_URL: 'mongodb://mongodb/meteor',
MONGO_OPLOG_URL: 'mongodb://mongodb/local'
MONGO_OPLOG_URL: 'mongodb://mongodb/local',
MAIL_URL: ''
Copy link
Owner

Choose a reason for hiding this comment

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

Do you know if mailing works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, mailing works (I tested invinting users to an application)


<p>2. {{{i18n 'app.config_connect_code'}}}</p>
<div id="no-data-code-wrap">
<ul class="nav nav-tabs" role="tablist">
Copy link
Owner

Choose a reason for hiding this comment

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

Actually I like to have the option to set the apm settings via code or env. So I would not remove these tabs here and better add the apm engine url to the other tabs too:
Code:
Kadira.connect('{{state$appId}}', '{{state$secret}}', { endpoint: apmEngineUrl });
For env:
KADIRA_OPTIONS_ENDPOINT (https://github.com/lmachens/kadira/blob/master/lib/environment_variables.js#L67)

@vparpoil
Copy link
Contributor Author

I just updated with the two other options to connect an app, thanks for your feedback, and sorry for the bad indentation on commit 023d8c7, I fixed it

Copy link
Owner

@lmachens lmachens left a comment

Choose a reason for hiding this comment

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

thx! I will add prettier soon

@lmachens lmachens merged commit bb1091f into lmachens:master May 17, 2018
lmachens added a commit that referenced this pull request May 17, 2018
@lmachens
Copy link
Owner

I added apmEngineUrl to the settings dialog too.
see e02f924

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