GreenStepsChatt / greensteps

This is the web app for Green Steps, a community focused initiative that incentivizes litter cleanup in the Chattanooga area.
https://www.greenstepschatt.com/
MIT License
12 stars 31 forks source link

Remove lint:factory_girl task #141

Closed crawfoal closed 6 years ago

crawfoal commented 6 years ago

I decided to lint the factories in a spec rather than in a rake task and I forgot to delete the task file. It would be a good idea to double check and make sure that the task isn't being called anywhere :)

alvaroscelza commented 6 years ago

Hi! I'd like to try this one, it's my first time contributing to open source so I'll probably be needing help to set everything up.

crawfoal commented 6 years ago

@Loaderon - Your first contribution! How exciting! Let me know what questions you have 🙂

alvaroscelza commented 6 years ago

Hi, I've been learning a lot about Ruby and ROR lately. I finished many tutorials and started reading your project's code. I've got many questions, I would be very thankful if you took some time to help me with them. Arm yourself with patience, I don't consider myself to be very good at this yet :/

1) I've some trouble running the development setup configuration: When running ruby bin/setup (I use Windows) it outputs:

== Installing dependencies ==
The Gemfile's dependencies are satisfied

== Preparing database ==

== Command ["bin/rails db:setup"] failed ==

Yet if I run only ruby bin/rails db:setup it seems to work correctly:

Database 'greensteps_development' already exists
Database 'greensteps_test' already exists
-- enable_extension("plpgsql")
   -> 0.0466s
-- create_table("addresses", {:force=>:cascade})
   -> 0.0391s
-- create_table("deeds", {:id=>:serial, :force=>:cascade})
   -> 0.0161s
-- create_table("prizes", {:id=>:serial, :force=>:cascade})
   -> 0.0156s
-- create_table("roles", {:id=>:serial, :force=>:cascade})
   -> 0.0297s
-- create_table("stations", {:force=>:cascade})
   -> 0.0163s
-- create_table("task_records", {:id=>false, :force=>:cascade})
   -> 0.0102s
-- create_table("users", {:id=>:serial, :force=>:cascade})
   -> 0.0439s
-- create_table("users_roles", {:id=>false, :force=>:cascade})
   -> 0.0087s
-- add_foreign_key("deeds", "users")
   -> 0.0048s
-- enable_extension("plpgsql")
   -> 0.0484s
-- create_table("addresses", {:force=>:cascade})
   -> 0.0267s
-- create_table("deeds", {:id=>:serial, :force=>:cascade})
   -> 0.0201s
-- create_table("prizes", {:id=>:serial, :force=>:cascade})
   -> 0.0148s
-- create_table("roles", {:id=>:serial, :force=>:cascade})
   -> 0.0288s
-- create_table("stations", {:force=>:cascade})
   -> 0.0126s
-- create_table("task_records", {:id=>false, :force=>:cascade})
   -> 0.0105s
-- create_table("users", {:id=>:serial, :force=>:cascade})
   -> 0.0388s
-- create_table("users_roles", {:id=>false, :force=>:cascade})
   -> 0.0099s
-- add_foreign_key("deeds", "users")
   -> 0.0031s

Any ideas on what I may be doing wrong?

2) By following the development setup file, I ran into some trouble that finally managed to find out what I was missing. As a result, in order to help newbies just like me that also want to contribute with this project I would like to propose some modifications to the file, like adding the following explanation:

You have to set your postgresql user and password in the config/database.yml, like:
development:
  <<: *default
  database: greensteps_development
  username: yourUserName
  password: yourPassword

Before the

you should run bin/setup

line.

Is it correct to do so? Should I open an Issue to do that or directly modify the file in my fork and propose a Pull Request?

3) By analysing the project's code, and running rake --tasks I found out that .rake_tasks holds a line: lint:factory_girl I don't have yet the enough expertise so as to be sure that is what you wanted to avoid, since it is in .rake_tasks I assume it is still called as a rake task, just as you mentioned you wanted to avoid. Yet it has this "lint:" prefix... I will be learning about Lint, Rake, factories and Specs now, so as to be finally able to work directly in this issue, but I wanted to have a first approximation.

Thanks for your help! I have learnt a lot of things thanks to your project!

crawfoal commented 6 years ago

@Loaderon We've all been there - just getting started out and having a lot of questions. Ask away!

First off - you shouldn't have to type "ruby" before running commands like bin/setup and bin/rails db:setup. Have you tried running the commands without the "ruby" part?

1 - Did it give you any details after it said that the rake task failed?

2 - Thanks for pointing this out!

Typically you want to do separate issues and pull requests for unrelated issues but in this case, the fixes for each issue are so small that I think it is fine to lump them together 🙂

Your proposed changes look right, but it would still be good to test them out. In this case, you should be able to do that by cloning the repo into a new folder. This new folder would be like a throwaway test folder for testing the setup process. The folder your working out of right now (the one that is tied to your fork) would be the one that you use to commit your actual changes and to push to GitHub. You'd delete the new test / throwaway folder after you're done with this PR. Does this all make sense?

3 - I think .rake_tasks is just a cache of the names of the rake tasks that the project supports. I think it used by ZSH (a type of shell script, like BASH) for tab completion. It should be fine to just delete that line.

lint is a namespace that I've used to group all lint related factories.

If you've gotten deep into learning about linting and testing, you might feel like "aw man... I spent all of this time learning about this stuff and Amanda is saying I just need to delete that line - or not even do anything with it!" But testing is super important! You will soon be thankful for that knowledge and put it to use 🙂