Page MenuHomekolab.org

Better structure for console commands, and unification of code/behavior
Needs RevisionPublic

Authored by vanmeeuwen on Oct 5 2020, 11:45 AM.

Details

Reviewers
machniak
Group Reviewers
Restricted Project
Summary

Merge branch 'master' into arcpatch-D1684

Standardize downloading a file

Demonstrate command restructuring with magic

Test Plan

./phpunit (tests not updated)

Diff Detail

Repository
rK kolab
Branch
arcpatch-D1702
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 31693
Build 12043: arc lint + arc unit

Event Timeline

vanmeeuwen requested review of this revision.Oct 5 2020, 11:45 AM
vanmeeuwen created this revision.
vanmeeuwen updated this revision to Diff 4528.Oct 5 2020, 11:52 AM
  • Further reduce duplication
vanmeeuwen updated this revision to Diff 4534.Oct 5 2020, 1:32 PM
  • show-case implementation of 'relations'
vanmeeuwen updated this revision to Diff 4540.Oct 5 2020, 6:23 PM
  • Commentary and documentation
  • user:wallets translation to the new system
  • More magic
vanmeeuwen updated this revision to Diff 4546.Oct 5 2020, 7:05 PM
  • And... another iteration to experiment with.
vanmeeuwen updated this revision to Diff 4576.Oct 6 2020, 12:48 PM
  • Some create object logic
vanmeeuwen updated this revision to Diff 5008.Nov 9 2020, 2:14 PM

Rebase on master

  • Some preliminary export commands
  • Add wallet:balances command
  • Merge branch 'master' into arcpatch-D1702
  • Move some commands to a scalpel namespace, add generic ObjectUpdateCommand
vanmeeuwen updated this revision to Diff 5014.Nov 9 2020, 2:50 PM
  • Move some more commands around
machniak requested changes to this revision.Wed, Dec 23, 11:35 AM
machniak added a subscriber: machniak.
machniak added inline comments.
src/app/Console/ObjectCreateCommand.php
76

This is redundant. create() method already returns the object on success.

src/app/Console/ObjectListCommand.php
42

You miss command prefix here.

src/app/Console/ObjectRelationListCommand.php
118

I think instead of ->each() you can use foreach for all above types and array. Also, \Illuminate\Database\Eloquent\Relations\Relation does not implement each() method itself, so I guess the check is not right. I'm not sure but mabe we could use is_countable() here as replacement for these three IFs.

src/app/Console/ObjectUpdateCommand.php
93

Should we have an error printed here?

src/app/Domain.php
435

I think $this->error() in here is not correct. Also the TODO above.

This revision now requires changes to proceed.Wed, Dec 23, 11:35 AM