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

v4 (major update) #7

Merged
merged 27 commits into from
Jan 31, 2020
Merged

v4 (major update) #7

merged 27 commits into from
Jan 31, 2020

Conversation

tarampampam
Copy link
Contributor

@tarampampam tarampampam commented Jan 30, 2020

Q A
Bug fix? No
New feature? Yes

Description

Changed

  • Minimal illuminate\* packages version now is ^5.5
  • Maximal illuminate\* packages version now is ~6.0
  • Interface References\ReferenceInterface now extends IteratorAggregate, Countable, Illuminate\Contracts\Support\Arrayable and not contains any methods
  • Package service-provider don't use cache for performance optimization
  • Performance improvements in has* methods (indexes used)

Added

  • New reference implementations:
    • References\CadastralDistricts
    • References\SubjectCodes
    • References\VehicleCategories
    • References\VehicleFineArticles
    • References\VehicleRegistrationActions
    • References\VehicleRepairMethods
  • New reference:
    • References\VehicleTypes
  • New interface:
    • References\Entities\EntityInterface (extends Illuminate\Contracts\Support\Support\Arrayable)
  • New entity classes:
    • References\Entities\CadastralArea
    • References\Entities\CadastralDistrict
    • References\Entities\SubjectCodesInfo
    • References\Entities\VehicleCategory
    • References\Entities\VehicleFineArticle
    • References\Entities\VehicleRegistrationAction
    • References\Entities\VehicleRepairMethod
    • References\Entities\VehicleType
  • GitHub actions for a tests running

Removed

  • Facades (AutoCategoriesFacade, AutoFinesFacade, AutoRegionsFacade, CadastralRegionsFacade, RegistrationActionsFacade, RepairMethodsFacade)
  • Classes:
    • References\AbstractReference
    • References\AbstractReferenceEntry
    • References\AutoCategories\AutoCategories
    • References\AutoCategories\AutoCategoryEntry
    • References\AutoFines\AutoFineEntry
    • References\AutoFines\AutoFines
    • References\AutoRegions\AutoRegionEntry
    • References\AutoRegions\AutoRegions
    • References\CadastralDistricts\CadastralDistrictEntry
    • References\CadastralDistricts\CadastralDistricts
    • References\CadastralDistricts\CadastralRegionEntry
    • References\CadastralDistricts\CadastralRegions
    • References\RegistrationActions\RegistrationActionEntry
    • References\RegistrationActions\RegistrationActions
    • References\RepairMethods\RepairMethods
    • References\RepairMethods\RepairMethodsEntry
  • Interfaces:
    • References\ReferenceEntryInterface
  • Traits:
    • References\Traits\TransliterateTrait

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I wrote unit tests for my code
  • I have made changes in CHANGELOG.md file

@tarampampam tarampampam self-assigned this Jan 30, 2020
@codecov-io
Copy link

codecov-io commented Jan 30, 2020

Codecov Report

Merging #7 into master will increase coverage by 6.32%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##             master     #7      +/-   ##
==========================================
+ Coverage     93.67%   100%   +6.32%     
+ Complexity      249    111     -138     
==========================================
  Files            24     16       -8     
  Lines           411    251     -160     
==========================================
- Hits            385    251     -134     
+ Misses           26      0      -26
Impacted Files Coverage Δ Complexity Δ
src/References/SubjectCodes.php 100% <100%> (ø) 13 <13> (?)
src/References/VehicleRegistrationActions.php 100% <100%> (ø) 10 <10> (?)
src/References/VehicleRepairMethods.php 100% <100%> (ø) 10 <10> (?)
src/References/Entities/VehicleCategory.php 100% <100%> (ø) 4 <4> (?)
src/References/Entities/CadastralArea.php 100% <100%> (ø) 4 <4> (?)
src/References/Entities/VehicleType.php 100% <100%> (ø) 6 <6> (?)
src/References/CadastralDistricts.php 100% <100%> (ø) 10 <10> (?)
src/References/VehicleTypes.php 100% <100%> (ø) 9 <9> (?)
src/References/VehicleCategories.php 100% <100%> (ø) 9 <9> (?)
src/ServiceProvider.php 100% <100%> (ø) 1 <0> (-7) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8db9662...f8c1969. Read the comment docs.

@tarampampam tarampampam changed the title WIP: v4 (major update) v4 (major update) Jan 30, 2020
Copy link
Contributor

@eldario eldario left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Только так

$this->code = $code;
$this->name = $name;

foreach ($areas as $area) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему foreach не заменить на

$this->areas = $areas;

а в CadastralDistricts сразу не прописать ключи?

Copy link
Contributor Author

@tarampampam tarampampam Jan 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because $this->areas used as index, where key is area code, and value is area object (areas order is not important in this case)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved

$areas = [];

foreach ($datum['areas'] as $area_info) {
$areas[] = new CadastralArea($area_info['code'], $area_info['name']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему не

$areas[$area_info['code']] = new CadastralArea($area_info['code'], $area_info['name']);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Разделение ответственности отсутствие зависимости от порядка (ключей) для CadastralDistrict в array $areas

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved

{
return (string) Json::encode($this->toArray(), $options);
}
}

This comment was marked as resolved.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You always can do something like this:

\count($district->getAreas());

keep it simple =)


// Multiple dots replace with single
return (string) \preg_replace('/\.+/', '.', $value);
}

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

*/
public function __construct(StaticReferenceInterface $static_reference)
{
$counter = 0;

This comment was marked as resolved.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be

foreach (\array_values((array) $static_reference->getData(true)) as $key => $datum) {
    $this->entities[$key] = new VehicleRegistrationAction($datum['codes'], $datum['description']);
    ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не думаю, что здесь это на что-либо влияет (пример @Reallife потребует дополнительный вызов array_values)

}
}
$this->app->singleton(CadastralDistricts::class, static function (): CadastralDistricts {
return new References\CadastralDistricts(StaticReferencesData::cadastralDistricts());

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нет, по запаре

parent::setUp();

/** @var m\MockInterface|StaticReferenceInterface $static_reference */
$static_reference = m::mock(StaticReferenceInterface::class)

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. выносить в абстракт 2 строчки?
  2. какие данные и из каких пропертей?

@tarampampam tarampampam merged commit fa3794d into master Jan 31, 2020
@tarampampam tarampampam deleted the regular-update branch January 31, 2020 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants