lightbend / mesos-spark-integration-tests

Mesos Integration Tests on Docker/Ec2
16 stars 9 forks source link

Attributes support per slave #13

Closed skonto closed 9 years ago

skonto commented 9 years ago
skonto commented 9 years ago

i will have to rebase for this...

skyluc commented 9 years ago

In cfg.sh, only one of the methods is used. Unused code becomes fast "legacy" code.

Also the function get_number_of_slaves should be used. It would remove the requirement to specify both --number-of-slaves and --slaves-cfg-file, and reduce the risks of misconfigurations.

skyluc commented 9 years ago

A few comments, but otherwise it seems to work fine.

skonto commented 9 years ago

re-based it needs a few mods to fix issues...

skyluc commented 9 years ago

You should really use the git rebase command, instead of git merge.

Basically:

git rebase master
skonto commented 9 years ago

i did that its just the push force which makes the difference...

skyluc commented 9 years ago

LGTM. You can squash and merge :smiley:

skonto commented 9 years ago

i think all is in one place now.. :) check and merge pls...

skyluc commented 9 years ago

:+1: good to merge

dragos commented 9 years ago

Not anymore. Guys, if there's an LGTM please merge, there's no reason to wait and get into this sort of issues.

skonto commented 9 years ago

but now have to resolve conflicts again :(

dragos commented 9 years ago

Exactly! That's why it makes no sense to drag these PRs as 'open' once they have been LGTM'd. :) Sorry about that.