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

[WIP] Management/Admin scripts for the project #971

Merged
merged 7 commits into from
Apr 4, 2016
Merged

Conversation

magsout
Copy link
Member

@magsout magsout commented Mar 17, 2016

r? @karlcow

wip, just for starting cleaned root folder, what do you think ?

  • removed Makefile because it's device agnostic (unix only) and add all recipes directly in package.json I like this convention to add all recipes in one place
  • removed .eslint and added all rules in package.json
  • moved requirement.txt for now in config folder
  • removed webcompat.sh

@karlcow
Copy link
Member

karlcow commented Mar 17, 2016

ah interesting. One thing I don't know the answer is that if there is a constraint for requirements.txt to be in the root directory. apart of that it's fine for me.

@magsout once you moved requirements.txt, does the project restart well? I wonderf what is happening for example with
https://github.com/webcompat/webcompat.com/blob/master/run.py#L71

@magsout
Copy link
Member Author

magsout commented Mar 17, 2016

maybe just managed path here https://github.com/webcompat/webcompat.com/blob/master/run.py#L77 ? no ? Let me test.. Honestly I ddin't test anyhting about it, recipes only ... 🐰

@magsout
Copy link
Member Author

magsout commented Mar 17, 2016

@karlcow I modified path in run.py and that's ok

screen recording 2016-03-17 at 08 51 pm

@magsout magsout force-pushed the script-flat branch 2 times, most recently from cc3ddf0 to 7157178 Compare March 17, 2016 21:02
@miketaylr
Copy link
Member

Moving requirements.txt shouldn't be an issue -- we'll just need to update CONTRIBUTING.md docs to point to the new path when using pip install -r path/to/requirements.txt

@miketaylr
Copy link
Member

But +1 from me, I've been using npm scripts in another small project.

@@ -36,7 +36,7 @@ before_install:
install:
- travis_retry npm install -g grunt-cli
- travis_retry npm install
- travis_retry pip install -r requirements.txt --download-cache $HOME/.pip-cache
- travis_retry pip install -r config/requirements.txt --download-cache $HOME/.pip-cache

This comment was marked as abuse.

@miketaylr
Copy link
Member

@magsout the docs that need to be updated are here: https://github.com/webcompat/webcompat.com/blob/master/CONTRIBUTING.md#installing-project-source-code

r+ from me. Let's let @karlcow give the final +1.

@magsout
Copy link
Member Author

magsout commented Mar 23, 2016

Need to figure out how npm travis script can be used to test the installation works well. perhaps decompose even in the script package.json

@magsout the docs that need to be updated are here: https://github.com/webcompat/webcompat.com/blob/master/CONTRIBUTING.md#installing-project-source-code

Yes, of course, I waited that my idea looks good for you ;)

@karlcow
Copy link
Member

karlcow commented Mar 24, 2016

@magsout yup looks good.

I'm not a big fan of having to use package.json for the recipes, but I can live with it. :) I would have personally used something in python, but not very important.
How is it used now that the Makefile is gone?

Thanks! 🍰

@magsout
Copy link
Member Author

magsout commented Mar 27, 2016

I'm not a big fan of having to use package.json for the recipes, but I can live with it. :) I would have personally used something in python, but not very important.
How is it used now that the Makefile is gone?

In many project with have node and package.json, So I guess wen can say it's universal ? ;p

How is it used now that the Makefile is gone?

just replace all make by npm run

npm run watch
npm run build
npm run start

@magsout
Copy link
Member Author

magsout commented Mar 28, 2016

Updated @karlcow @miketaylr

What do you think about all recipes ? Revelant ? Clear ?

@@ -38,5 +38,64 @@
"postcss-url": "^5.1.1",
"suitcss-utils-align": "^0.2.0",
"suitcss-utils-display": "^0.4.0"
},
"scripts": {
"init": "npm run module && npm run virtualenv && && npm run pip && npm install && npm run config",

This comment was marked as abuse.

@miketaylr
Copy link
Member

Yep, LGTM except for the one small && issue.

@karlcow
Copy link
Member

karlcow commented Mar 28, 2016

@magsout looks good to me.
Thanks :)

@magsout
Copy link
Member Author

magsout commented Mar 30, 2016

OOoOps

@miketaylr
Copy link
Member

@magsout looks good to me.

Karl gave his r+. So let's merge.

@miketaylr miketaylr merged commit deefd94 into master Apr 4, 2016
@miketaylr miketaylr deleted the script-flat branch April 4, 2016 01:57
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.

3 participants