search

Genki-S / ttnt

Test This, Not That! (Google Summer of Code 2015 project under Ruby on Rails)
MIT License
263 stars 7 forks source link

Make storage file rakefile bound #34

Closed Genki-S closed 9 years ago

Genki-S commented 9 years ago

Implementation of #32

I made .ttnt storage file resides along with Rakefile.

https://github.com/Genki-S/ttnt/commit/2404ccdba91e9ba3723f29c85e0222ff44ed3112 introduces TTNT.root_dir method which points to the directory of nearest Rakefile, and use it for storage file name.

I also changed paths in test-to-code mappings to start from TTNT.root_dir, so that irrelevant files from other gems in the same repository can be excluded from mappings. https://github.com/Genki-S/ttnt/commit/acdf4b877492d94d0256a05d841848bc3072eb38 (the use of TTNT.root_dir in select_project_files is the point)

And as .ttnt can now resides not only in repository root but also in subdirectories, I had to change the logic of reading the contents of .ttnt from git history. I added a logic which walks through git tree searching .ttnt in subdirectories: https://github.com/Genki-S/ttnt/commit/15d5fa17a0277d6e164e0d3e44c4513ec52953ef

Finally, I wrote integration test for changes in this PR (https://github.com/Genki-S/ttnt/commit/048c36561b2324087ab04cf0d05f67aa01c24ec6, https://github.com/Genki-S/ttnt/commit/08a07f03f3a5d686e58531db06cb55e298f77130).

@robin850 could you review the code please?

Genki-S commented 9 years ago

Additionally, I changed TTNT.root_dir to cache the result: https://github.com/Genki-S/ttnt/commit/81b72bdc28934c9b574c0cdbef54c3417fc3aea8

robin850 commented 9 years ago

Really cool stuff here!

I'm not a huge fan of the name ttnt_module.rb — maybe you could rename it paths.rb or maybe internals.rb ; you will certainly not only have file-system-related methods. What do you think ?

Genki-S commented 9 years ago

I'm not a huge fan of the name ttnt_module.rb — maybe you could rename it paths.rb or maybe internals.rb ; you will certainly not only have file-system-related methods. What do you think ?

I agree. I took the name from rake/rake_module.rb without thinking much. I took the internals.rb idea, thank you! :smile: https://github.com/Genki-S/ttnt/commit/e6ecd8f415f1033ec91964a3c8b3a3f9697995c8

(P.S. I rebased all commits to resolve conflict)

robin850 commented 9 years ago

Is this correct?

Yep! More simply, you are using a variable inside a class method so you should rather rely on a class variable. :p

Awesome, let's merge this! :clap:

Genki-S commented 9 years ago

More simply, you are using a variable inside a class method so you should rather rely on a class variable.

Oh I see :eyes:

Thank you for merging! :tada: