CI, better tests, bugfixes and cleaning #103

yurii-github · 2020-10-25T16:10:59Z

some current ideas I have to improve.

namespaces
phpunit version
some builds for different PHP versions

I wonder what do you think about it. If you don't mind I can keep this PR open like work in progress.

topclaudy · 2020-10-25T17:01:35Z

Why do you close this?

yurii-github · 2020-10-25T17:48:05Z

I just wanted a feedback, if it is ok I can keep PR open. I have some ideas to make tests better and drop some dependencies, because in my view this package can live without them.

yurii-github · 2020-10-25T17:49:27Z

by the way, builds for Laravel 5.6 are failing in master due to illuminate/database requirement (as I understand)
regards

topclaudy · 2020-10-25T19:28:54Z

by the way, builds for Laravel 5.6 are failing in master due to illuminate/database requirement (as I understand)
regards

I just wanted a feedback, if it is ok I can keep PR open. I have some ideas to make tests better and drop some dependencies, because in my view this package can live without them.

Every contribution for improvements is accepted

topclaudy · 2020-10-25T19:30:04Z

I just wanted a feedback, if it is ok I can keep PR open. I have some ideas to make tests better and drop some dependencies, because in my view this package can live without them.

Which dependencies do you want to drop?

topclaudy · 2020-10-25T19:30:12Z

by the way, builds for Laravel 5.6 are failing in master due to illuminate/database requirement (as I understand)
regards

We can bump the minimum Laravel version (-> 5.7)

yurii-github · 2020-10-26T01:41:34Z

as sum up of my audit of the code

lluminate database until version 5.6 is broken with your code and cannot be fixed due to laravel refactoring. In fact, from 5.6 till 5.8 we have similar iissue but we can work with workaround.
disassociate() and some other methods are not implemented thus keep potential issues.
as 5.5 cannot be not supported we can up PHP requirements to 7.1.

what i have done

i have dropped support of old versions and upped to 5.6 , and PHP to 7.1
i have moved tests in namespace
i have added support for several phpunit versions
i have added travis ci as backbone for ci
i have added semi-full support matrix for illuminate database
i have removed dependency on phpunit as it is not recommended
i have dropped all other non required dependencies

latest build https://travis-ci.org/github/yurii-github/compoships/builds/738894383

i wonder what you say. only after we agree or disagree on further move/plans I continue with morph support.

see you

p.s. about dropping support up to 5.7 - for me it is vital to keep 5.6 support as I personally depend on it in one way or another.
p.p.s. i will do rebase tomorrow

yurii-github · 2020-10-26T01:54:30Z

src/Database/Eloquent/Relations/BelongsTo.php

+            $this->child->setAttribute($foreignKey, $value);
+        }
+        // BC break in 5.8 : /~https://github.com/illuminate/database/commit/87b9833019f48b88d98a6afc46f38ce37f08237d
+        $relationName = property_exists($this, 'relationName') ? $this->relationName : $this->relation;


maybe throw error if 'relation' does not exist too

topclaudy · 2020-10-27T18:09:06Z

Sorry for the delay. I'm very busy on weekdays. Thanks for the improvements. I'll check them during the weekend and give feedback

yurii-github · 2020-10-28T22:30:22Z

hello
sure, no problems. I have almost finished with testing setup. I have added codeclimate as code coverage, so you just need to setup your own keys on travis ci for CC_TEST_REPORTER_ID

5.4 is not supported by your package. So we don't need 5.6 PHP support too /~https://github.com/laravel/framework/pull/20871/files
bc break in 5.8 i have fixed illuminate/database@87b9833

I will add some more tests to cover my cases, and that's probably it.
p.s. I would also recommend to remove Model class as trait usage is sufficient.

https://travis-ci.com/github/yurii-github/compoships/builds/195171843

- added Travis CI support - added Codeclimate support - better PHPUnit config

- made tests as namespaced - fix composer.json with proper dependencies - drop support for service provider hook as it is bloatware - added support for several PHPUnit versions

yurii-github · 2020-11-01T16:43:46Z

I think I did around 90% of what I wanted initially. Along this journey I have found several issues too.
anyway, test coverage looks like this by now.

p.s. i will squash commits tomorrow

topclaudy · 2020-11-01T16:50:58Z

Wow!! That's a lot of contributions. Thanks!

topclaudy · 2020-11-01T16:56:26Z

Can you please update the README for the steps required to run the tests?

yurii-github · 2020-11-02T16:04:49Z

good day
sure, but give me a week or so, I have some very high work load this week, maybe I delay it till this weekend.

yurii-github · 2020-11-15T00:42:30Z

I have finished with my changes.
I will be glad to hear your feedback on code review etc.
If you find it is too big, we can split it in small PRs and I can do it that way, or whatever you think is good.

I have unresolved only 1 thing - I don't know how to forbid user to install illuminate/database 5.6 with PHP 7.3+, because it is laravel issue. maybe some note in readme is enough, I don't know

Also, there is place for improvement like test coverage, because this project is still very risky in sense of changes and tweaks it provides.

regards

FINAL CHANGELOG

Changes

dropped requirement for PHP version, it is enforced by illuminate/database package
upped support for illuminate/database to 5.6+ (it requires PHP 7.1+ https://packagist.org/packages/illuminate/database#5.6.x-dev)
dropped hard dependencies on framework and other useless packages
removed Model class as it is not needed
removed ComposhipsServiceProvider as it is not needed
developer now requires to obtain PHPUnit phar manually

Fixes

fixed several bugs for illuminate/database 5.6
fixed several bugs for illuminate/database 5.8

Tests

added namespaces to tests so they can be resolved with autoload
added support for different PHP versions (7.1+)
added TravisCI support
added Codeclimate support
added more tests to cover more edge cases
other minor fixes to tests

…e package

topclaudy · 2020-11-26T20:04:09Z

I have finished with my changes.
I will be glad to hear your feedback on code review etc.
If you find it is too big, we can split it in small PRs and I can do it that way, or whatever you think is good.

I have unresolved only 1 thing - I don't know how to forbid user to install illuminate/database 5.6 with PHP 7.3+, because it is laravel issue. maybe some note in readme is enough, I don't know

Also, there is place for improvement like test coverage, because this project is still very risky in sense of changes and tweaks it provides.

regards

FINAL CHANGELOG

Changes

dropped requirement for PHP version, it is enforced by illuminate/database package

upped support for illuminate/database to 5.6+ (it requires PHP 7.1+ https://packagist.org/packages/illuminate/database#5.6.x-dev)

dropped hard dependencies on framework and other useless packages

removed Model class as it is not needed

removed ComposhipsServiceProvider as it is not needed

developer now requires to obtain PHPUnit phar manually

Fixes

fixed several bugs for illuminate/database 5.6

fixed several bugs for illuminate/database 5.8

Tests

added namespaces to tests so they can be resolved with autoload

added support for different PHP versions (7.1+)

added TravisCI support

added Codeclimate support

added more tests to cover more edge cases

other minor fixes to tests

Very nice! I'll give you feedback before next week.

topclaudy · 2020-11-26T20:37:19Z

All good! There are some unused variables in the tests but that's not an issue (we can clear them later).
How would you suggest to handle tests/Unit/HasManyTest:broken_Compoships_hasOneOrMany_whereInMethod__missingRelationColumn to make it complete? We can put this warning in the doc as a workaround.

yurii-github · 2020-11-27T16:03:31Z

hello. thank you for the feedback

All good! There are some unused variables in the tests but that's not an issue (we can clear them later).
How would you suggest to handle tests/Unit/HasManyTest:broken_Compoships_hasOneOrMany_whereInMethod__missingRelationColumn to make it complete? We can put this warning in the doc as a workaround.

I guess it is directly related to this issue #98
in my view, we should enforce selection of such fields in implicit way somewhere in with() or in some final query builder method.
I can do it in new PR to target that particular issue. So warning is probably ok?

in any case, I have granted you write access to this PR branch, you can change everything you want and I will continue on master after.

regards

topclaudy · 2020-11-27T16:44:37Z

Merged! You can go ahead for the PR regarding #98

yurii-github · 2020-11-27T17:19:11Z

thanks.

Small question.. how do I fit code into this styleci.io ? is there any local tool to autofix it?

topclaudy · 2020-11-27T17:23:47Z

thanks.

Small question.. how do I fit code into this styleci.io ? is there any local tool to autofix it?

It provides a diff/patch file you can apply to the code using the git apply [DIFF_FILE] command.

yurii-github · 2020-11-27T17:25:35Z

I see. thank you

yurii-github closed this Oct 25, 2020

yurii-github reopened this Oct 25, 2020

yurii-github commented Oct 26, 2020

View reviewed changes

yurii-github added 2 commits October 29, 2020 00:39

CI/CD integrations

7bdb694

- added Travis CI support - added Codeclimate support - better PHPUnit config

fix unit tests

01967f9

- made tests as namespaced - fix composer.json with proper dependencies - drop support for service provider hook as it is bloatware - added support for several PHPUnit versions

yurii-github force-pushed the better-tests branch from f1267ae to 53dc79d Compare October 28, 2020 22:46

yurii-github added 4 commits November 15, 2020 01:56

clean code

698ea8e

added BC fix for 5.6~5.8

2cd54b6

drop Model support as it is not needed

fe99add

better test coverage

4ebd604

yurii-github force-pushed the better-tests branch from c51c380 to 4ebd604 Compare November 14, 2020 23:59

yurii-github added 2 commits November 15, 2020 02:20

update README.md

23684b8

revert change, but leave comment

1997a86

yurii-github changed the title ~~WIP: better tests~~ WIP: CI, better tests, bugfixes and cleaning Nov 15, 2020

yurii-github force-pushed the better-tests branch from d2d82d2 to c6bc098 Compare November 15, 2020 00:59

drop PHP version requirement, as it is enforced by illuminate/databas…

28ebcc0

…e package

yurii-github force-pushed the better-tests branch from c6bc098 to 28ebcc0 Compare November 15, 2020 01:02

yurii-github changed the title ~~WIP: CI, better tests, bugfixes and cleaning~~ CI, better tests, bugfixes and cleaning Nov 15, 2020

topclaudy merged commit 28ebcc0 into topclaudy:master Nov 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CI, better tests, bugfixes and cleaning #103

CI, better tests, bugfixes and cleaning #103

yurii-github commented Oct 25, 2020 •

edited

Loading

topclaudy commented Oct 25, 2020

yurii-github commented Oct 25, 2020 •

edited

Loading

yurii-github commented Oct 25, 2020

topclaudy commented Oct 25, 2020

topclaudy commented Oct 25, 2020

topclaudy commented Oct 25, 2020

yurii-github commented Oct 26, 2020 •

edited

Loading

yurii-github Oct 26, 2020

topclaudy commented Oct 27, 2020

yurii-github commented Oct 28, 2020 •

edited

Loading

yurii-github commented Nov 1, 2020

topclaudy commented Nov 1, 2020

topclaudy commented Nov 1, 2020

yurii-github commented Nov 2, 2020

yurii-github commented Nov 15, 2020 •

edited

Loading

topclaudy commented Nov 26, 2020

FINAL CHANGELOG

Changes

Fixes

Tests

topclaudy commented Nov 26, 2020

yurii-github commented Nov 27, 2020

topclaudy commented Nov 27, 2020

yurii-github commented Nov 27, 2020

topclaudy commented Nov 27, 2020

yurii-github commented Nov 27, 2020

CI, better tests, bugfixes and cleaning #103

CI, better tests, bugfixes and cleaning #103

Conversation

yurii-github commented Oct 25, 2020 • edited Loading

topclaudy commented Oct 25, 2020

yurii-github commented Oct 25, 2020 • edited Loading

yurii-github commented Oct 25, 2020

topclaudy commented Oct 25, 2020

topclaudy commented Oct 25, 2020

topclaudy commented Oct 25, 2020

yurii-github commented Oct 26, 2020 • edited Loading

yurii-github Oct 26, 2020

Choose a reason for hiding this comment

topclaudy commented Oct 27, 2020

yurii-github commented Oct 28, 2020 • edited Loading

yurii-github commented Nov 1, 2020

topclaudy commented Nov 1, 2020

topclaudy commented Nov 1, 2020

yurii-github commented Nov 2, 2020

yurii-github commented Nov 15, 2020 • edited Loading

FINAL CHANGELOG

Changes

Fixes

Tests

topclaudy commented Nov 26, 2020

FINAL CHANGELOG

Changes

Fixes

Tests

topclaudy commented Nov 26, 2020

yurii-github commented Nov 27, 2020

topclaudy commented Nov 27, 2020

yurii-github commented Nov 27, 2020

topclaudy commented Nov 27, 2020

yurii-github commented Nov 27, 2020

yurii-github commented Oct 25, 2020 •

edited

Loading

yurii-github commented Oct 25, 2020 •

edited

Loading

yurii-github commented Oct 26, 2020 •

edited

Loading

yurii-github commented Oct 28, 2020 •

edited

Loading

yurii-github commented Nov 15, 2020 •

edited

Loading