fix(project): move mailer files into proper directories#1676
fix(project): move mailer files into proper directories#1676
Conversation
| * Gruntfile causes an error and should contain no strings | ||
| * bin/server.js extracts "/", so it is excluded. | ||
| */ | ||
| exclude: /(node_modules|test|Gruntfile|grunttasks|bin|scripts|\.git|config)/, |
There was a problem hiding this comment.
Losing these excludes was another nice side-effect. None of these things pollute the same space as the mailer source any more.
| apiSecret: smsConfig.apiSecret | ||
| }) | ||
| var sendSms = P.promisify(nexmo.message.sendSms, { context: nexmo.message }) | ||
| var sendSms = P.promisify(nexmo.message.sendSms, nexmo.message) |
There was a problem hiding this comment.
Changed to work with the auth server's old version of bluebird. Can be put back when we fix #1674.
| "shrinkwrap": "npmshrink", | ||
| "prepush": "grunt quicklint && grunt templates && git status -s | (! grep 'M lib/senders/templates/')", | ||
| "postinstall": "scripts/download_l10n.sh", | ||
| "shrinkwrap": "npmshrink && npm run postinstall", |
There was a problem hiding this comment.
Without this explicit postinstall the mailer fails after you regenerate shrinkwrap.
| "fxa-shared": "1.0.4", | ||
| "google-libphonenumber": "2.0.10", | ||
| "grunt-nunjucks-2-html": "vitkarpov/grunt-nunjucks-2-html#1900f91a756b2eaf900b20", | ||
| "handlebars": "4.0.6", |
There was a problem hiding this comment.
Updated because nsp warnings. Seems to work fine.
| if (error.reason && error.reasonCode) { | ||
| message = `${message}: ${error.reasonCode} ${error.reason}` | ||
| } else if (error.stack) { | ||
| message = error.stack |
There was a problem hiding this comment.
This helped me debug a problem while I was making the changes, thought it earned the right to stick around.
.travis.yml
Outdated
| # Test fxa-auth-mailer | ||
| - cd mailer | ||
| - grunt templates | ||
| - npm test |
There was a problem hiding this comment.
We already run npm test with COVERALLS_REPO_TOKEN=vKN3jjhAOwxkv9HG0VBX4EYIlWLPwiJ9d npm test above ^^^
philbooth
force-pushed
the
phil/mailer-tidy
branch
from
cf000b5 to
2bd88be
Compare
|
translations work well 👍 |
3 participants
What have we here?
An attempt to impose a saner directory structure on the merged auth mailer files. Instead of everything living under a standalone
mailerdirectory, the source files are inlib, the test files are intest, the scripts are inscriptsand so on. Additionally, the dependencies and grunt tasks are merged in with the auth server's, so there's only one place for everything.Is this just to satisfy your OCD?
No! There are tangible benefits too:
mocha!tapis is dead!package.jsonand only onenpm-shrinkwrap.jsonand only oneGruntfile.js!libdoesn't have to..back up to the root when importing mailer modules!liband some not is eradicated!nspagainst mailer deps!What hasn't moved?
The notable thing I haven't tried to do is merge the config files, so
mailer/config.jsstill exists and is referenced in a few places. This is partly because I'm wary of messing up ops-related stuff but mostly because they duplicate lots of things using different names, and updating all of those references makes the diff even more of a mess. I figure it's better to try and get this in first, then deal with the config afterwards.Are you sure you didn't break anything?
Fairly sure. Obviously the code lints and the tests all pass.
grunt templatesstill works. Thesend-smsscript still sends SMS messages and thewrite-emails-to-diskscript writes identical emails to the ones generated before this change. However, the machine I'm working on isn't properly set up like my old one so I haven't managed to deploy this branch viafxa-devyet. If anyone else wants to check out this branch and give it a spin, that would be grand.Anything else?
On the whole, I've tried to make the changes touch as few lines as possible, out of consideration for you the reviewer. So please bear that in mind before you pile in with your bike-sheddy comment about
const,=>or indentation. 😄@mozilla/fxa-devs r?