37 ms·
You have some usernames and passwords (to what looks like production servers) sprinkled throughout your app. May need to change those ASAP, wipe your history, o
by vcherubini 13y ago
You have some usernames and passwords (to what looks like production servers) sprinkled throughout your app. May need to change those ASAP, wipe your history, or something.
Lots of suggestions:
* Don't write your own cron scripts, use Symfony Commands.
* Delete the Acme Bundle.
* Don't store your vendor directory in Git! Completely unnecessary and waste of 47MB.
* Don't commit parameters.yml, that's what parameters.yml.dist is for.
* Use a migration tool (Doctrine Migrations are fine) instead of one big SQL dump.
* Not sure what mongodb/example.php is for.
* Why is all the plivo stuff in web/plivo/ instead of being a callable controller?
* Are you manually generating your Doctrine entities?
* Don't commit your asset directories (they should be symlinks).
* No need to use PHPMailer (or include the entire project in your repository), Symfony comes with Swiftmailer which is great.
* Same with src/Phone, add that library through Composer (https://packagist.org/packages/practo/libphonenumber-for-php https://packagist.org/packages/practo/libphonenumber-for-php).
The design looks great, but I wouldn't trust a lot of this (especially after seeing how many hosts and usernames and passwords you left scattered about).
- elwell 13y ago> Don't store your vendor directory in Git! Completely unnecessary and waste of 47MB. I find that it simplifies versioning as well as making it so you don't have to download individual dependencies. What is your suggestion otherwise?
- SEJeff 13y agoIf you must... use submodules.
- meritt 13y agoThat's what composer is for
- markdown 13y agowhat is composer, and why do I have to learn it if it is completely unnecessary?
- Tomdarkness 13y agoThere is a composer.json and composer.lock file in the repository. All you'd need to do to download all the dependencies is have composer installed and run "composer install".
- MichaelGG 13y agoThus making your application build process require full Internet connectivity, and making it require that every dependency hosting site is working. Committing dependencies is a solid decision to avoid taking a dependency on a bunch of third parties.
- lstamour 13y ago"application build process" or simply the checkout process? I'd suggest your deployment process shouldn't include a public git repo others can change. Instead, use your own fork on Github, or alternatively host a git repo local to your deploy network. After all, you're trying to avoid dependencies on things like Github working/failing, right? ;-)
- rds2000 13y agoPackage library systems like this are mirrored on CDN's and often cached locally in the user's `$HOME` folder in some way (at least in the case of npm and bower). If someone's downloading your repo, why wouldn't they mind getting their dependencies the normal way? Also, there is the benefit of your git history not having to track vendor libs, and you being able to lock versions in. This is about convenience to yourself as a developer, as well as others. Feel no need to include vendor libraries :), it's ok.
- jarofgreen 13y agoI do this. I know what you are meant to do is check in your composer.json and composer.lock then do a composer install on the server. But then you have a external dependency on your install process on live servers, which is a terrible idea. To got around this, I've seen companies say "we have our own package mirror". Or you could just check in the vendor folder? Another reason I do it: My software package is used by people who aren't PHP programmers and I want them to be able to just grab the files and go with no fuss. I do appreciate having the vendor folder in git takes space and is a pain ... but I think the trade off is worth it.
- hoodoof 13y agoIf it works, does any of this matter right now? He's trying to make his business survive, not craft a work of technical art. Shouldn't he spend his time on marketing if the thing works?
- Gigablah 13y agoMaybe not right now, but you've got to take care of technical debt sometime, and a few of those points are quick wins.
- zobzu 13y agoactually, there's no big big items but opensourcing isnt magic, you want people to look at it. if the project looks nicely managed, its a lot more attractive :)
- rodgerd 13y agoHis business isn't going to survive very long if he keeps uploading security credentials where everyone can see them.
- scottybowl 13y agoNot my finest hour.. Lesson learnt - valid point, and I will make certain to be more security conscious moving forward.
- kjs3 13y agoHey man, thanks for existing. The amount of money I make from cleaning up after dipshits who think "just throw some spaghetti at the wall, we'll deal with making it sustainable and secure later" keeps me almost embarrassingly busy. Of course, I'm as often as not the last money they spend, because if that's how you're running your business, you're screwed anyway. Keep being ignorant; keep writing me checks.
- hoodoof 13y agoThe road to success is littered with crashed startups who just keep pumping out code when the time for marketing instead of coding came long ago.
- mikegirouard 13y agoThis is good advice. The only addition I'd give is to start changing your passwords… quickly.
- mise 13y agoHow would you synchronise between versioning the symlinked assets and the app itself? Or do you just keep all images, for example, until you're simply sure you don't need them anymore?
- scottybowl 13y agoThank you, this is fantastic feedback. Sorry for not applying sooner, but my account was blocked from making comments (apparently I was trying to post too frequently!). Passwords were changed the second you mentioned them, and we'll be making all of these changes as soon as possible.
- shimon_e 13y agoThis is the thing about open source. Any one can make these improvements. Maybe you could open github issues for these suggestions and someone could have a go at making the patches.
- Udo 13y agoSome of these are debatable. > Don't store your vendor directory in Git! Completely unnecessary and waste of 47MB. While 47MB is indeed a bit large, storing vendor libraries alongside your code makes sure you have a working version when you pull down the repo. Nothing more frustrating than hunting down why the code you just checked out doesn't work. > Don't commit your asset directories (they should be symlinks). That depends on what you mean by assets. If there are binary assets needed for the app to run, they must be included. Imagine downloading a web app with all the icons and textures missing. Not good. And if source files exist for these assets, they should be in the repo, too (like PSDs or Inkscape SVGs)
- vcherubini 13y agoAll of my repo's have a 'build-dev' script in the root of the repository (see here: https://github.com/brightmarch/picto/blob/master/build-dev https://github.com/brightmarch/picto/blob/master/build-dev). Run that after you pull it down and it builds the environment for you (minus setting up Vagrant). You should commit your asset files in the Bundle, but not the ones in web/bundles/ because those are created by copying or symlinking the assets in your Bundle.
- philjohn 13y agoor use composer, and commit composer.lock to ensure when composer install is run you'll get the dependency versions that are meant to ship with the release.