Codecov Report

Merging #31 into master will decrease coverage by 1.69%. The diff coverage is 0%.

@@            Coverage Diff            @@
##           master      #31     +/-   ##
=========================================
- Coverage   31.02%   29.33%   -1.7%     
=========================================
  Files          37       43      +6     
  Lines         867      917     +50     
=========================================
  Hits          269      269             
- Misses        598      648     +50

Impacted Files	Coverage Δ
src/Models/Task.php	`0% <0%> (ø)`
src/Listeners/AddApiAttributes.php	`0% <0%> (ø)`	:arrow_up:
src/Jobs/RemovePackage.php	`0% <0%> (ø)`
src/Listeners/AddApiControllers.php	`0% <0%> (ø)`	:arrow_up:
src/Jobs/ComposerJob.php	`0% <0%> (ø)`
src/Api/Controllers/ListTaskController.php	`0% <0%> (ø)`
src/Providers/ComposerEnvironmentProvider.php	`0% <0%> (ø)`	:arrow_up:
src/Repositories/ExtensionRepository.php	`0% <0%> (ø)`	:arrow_up:
src/Api/Serializers/TaskSerializer.php	`0% <0%> (ø)`
src/Jobs/RequirePackage.php	`0% <0%> (ø)`
... and 7 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 d53911a...4435a89. Read the comment docs.

luceos commented 7 years ago

How much of this is still open?

clarkwinkelmann commented 7 years ago

I still want to add a way for the user to clear history item and make that listing a bit prettier. We can't ship such a UI...

Also maybe someone could check my table structure, just so I'm sure it's all right 😇

luceos commented 7 years ago

The task model needs a property dates with executed_at in it so that it will be auto-cast to Carbon.

I've noticed a new way of firing command in your code, which includes a not-so DRY way of adding tasks. Wouldn't a property on the command (or the class name) be sufficient to log into the task? Perhaps stick most of that logic into the BaseCommand instead..

clarkwinkelmann commented 7 years ago

@Luceos there's already a started_at and finished_at attributes. What would executed_at represent ?

luceos commented 7 years ago

Okay, that's fine. But both need to be in $casts in the model for auto casting to Carbon.

clarkwinkelmann commented 7 years ago

@Luceos which includes a not-so DRY way of adding tasks

I refactored that into the PackageManager. I also moved some stuff out of the PackageManager into the ComposerJob, because it's only relevant to the execution of the job (which should be queued in the future). I'll let you have a look

clarkwinkelmann commented 7 years ago

Note to self: there are some changes I did not commit in BazaarPage.js that still ended up in the dist extension.js :see_no_evil: I'll push another commit with a clean dist file

clarkwinkelmann commented 7 years ago

I removed the WIP, I'm done with dev for this feature.

Some testing before merging would be a good idea I guess.

extiverse / bazaar

Log and show task history in the UI #31

Codecov Report