auto icon indicating copy to clipboard operation
auto copied to clipboard

Published 20 hours ago • verified publisher icon intuit

Feat add `afterRun` hook

Open jBouyoud opened this issue 2 years ago • 6 comments

What Changed

Add new afterRun hook triggered after each command.

Why

Because beforeRun exists ;-P

Todo:

  • [x] Add tests
  • [x] Add docs

Change Type

Indicate the type of change your pull request is:

  • [ ] documentation
  • [ ] patch
  • [x] minor
  • [ ] major

jBouyoud avatar Mar 24 '22 07:03 jBouyoud

Just like https://github.com/intuit/auto/pull/2180 , I didn't understand why ci check pr-check is failling with a 404.

jBouyoud avatar Mar 25 '22 16:03 jBouyoud

I think this would mean that we have to update anywhere we use the afterShipIt hook since right now we assume we are shipping something. This would also be a breaking change too since it could break people using the hook

hipstersmoothie avatar Apr 04 '22 21:04 hipstersmoothie

I think this would mean that we have to update anywhere we use the afterShipIt hook since right now we assume we are shipping something. This would also be a breaking change too since it could break people using the hook

This is not really a breaking change since the hook payload permit it and the purpose of this hook seems to be : Ran after the 'shipit' command has run. So all plugins must handle the payload properly. But of course if the intent of this hook was to be only called when a release is done 🤷 .

I see two options :

  1. Check all existing afterShipIt hooks to properly handle newVersion === undefined
  2. Create a new hooks afterRun, that can be called after all commands (just like beforeRun). And change newVersion to only a string and update afterShipIt description to intent this.

I'll be glad to handle this. Please let me known what is your preference ?

jBouyoud avatar Apr 05 '22 08:04 jBouyoud

@jBouyoud Route 2 sounds like a great option! I like how we would then have the beforeRun/afterRun combo.

hipstersmoothie avatar May 20 '22 19:05 hipstersmoothie

@hipstersmoothie please find an implementation proposal for the route 2

jBouyoud avatar May 31 '22 09:05 jBouyoud

@hipstersmoothie anything that I can do to help merge this PR ?

jBouyoud avatar Aug 11 '22 07:08 jBouyoud

@hipstersmoothie I also rebase this PR ;-P

jBouyoud avatar Feb 08 '23 11:02 jBouyoud

pr-check, release seems fails because : PR env variable is empty ; probably related to this.

And github actions permissions are all in read even the workflow file specify write permissions 🤷

jBouyoud avatar Feb 09 '23 09:02 jBouyoud

@hipstersmoothie Let me know if everything miss to merge this

jBouyoud avatar Feb 10 '23 16:02 jBouyoud

:rocket: PR was released in v10.42.0 :rocket:

github-actions[bot] avatar Feb 10 '23 17:02 github-actions[bot]