search

ontodev / droid

DROID Reminds us that Ordinary Individuals can be Developers
BSD 3-Clause "New" or "Revised" License
4 stars 1 forks source link

Run `make clean` before deleting a branch #50

Closed beckyjackson closed 3 years ago

beckyjackson commented 4 years ago

~After clicking "Delete" next to a branch name, the button still says "Delete", not "Checkout" and the branch is not removed from the workspace. When you click the branch name next to "Delete" (after you've tried to delete it), it takes you to the GitHub branch page instead of the DROID branch page.~ I'm going to open a new issue in regards to this to keep the bug & enhancement separate.

~Furthermore, before deleting a branch, if there is a .cogs directory in that branch, we should probably run cogs delete -f somehow (or some equivalent...) so that the Google Sheet isn't out there forever. Otherwise, if we just remove the branch from the workspace, the sheet still exists and the user can't delete it manually because the service account is the owner.~

It would be good to run make clean when a branch is deleted.

jamesaoverton commented 4 years ago

DROID should not know anything specific about COGS, but it would be good for DROID to be able to do some clean-up before deleting a branch.

What if before deleting a branch DROID suggested running make clean? When we use COGS we could have the clean task run cogs delete --force or something.

beckyjackson commented 4 years ago

I think running make clean would be perfect.

jamesaoverton commented 4 years ago

Some documentation on conventions for Make targets -- there aren't many, but clean is one:

https://www.gnu.org/software/make/manual/html_node/Standard-Targets.html#Standard-Targets

jamesaoverton commented 3 years ago

@lmcmicu Can you please look into implementing this? I think this would be ideal, but maybe too fancy:

  1. the user clicks "Delete"
  2. DROID checks whether a make clean task exists
  3. if a make clean task exists, then below the confirmation message "Are you sure you want to delete the branch foo?" we add a checkbox which is checked by default: "Run make clean before deleting"
  4. if that checkbox is checked and the user clicks "Yes", then
    1. start running make clean, ignore any errors, and when it's complete delete the branch
    2. change the branch "Delete" button to a "Deleting..." message?

Maybe you can think of improvements.

lmcmicu commented 3 years ago

I am wondering why 4.ii is needed. Do we expect that the clean may take a long time?

jamesaoverton commented 3 years ago

The use case above is a Google Sheets API call we've seen take 10 seconds. I'm slightly worried about the branch being in an ambiguous state between present and absent. But I'm not sure it's worth the trouble.

lmcmicu commented 3 years ago

Ok. I don't think it will be too much trouble to change the button. But just to be clear, it is ok for the make clean to run in the foreground, correct? (I.e. no output to the console; from the user's point of view it will simply look like the browser tab is loading.)

jamesaoverton commented 3 years ago

I was thinking about running it in the background when I wrote my list. But foreground is simpler so let's see if that's good enough. Then there's no point in changing the button. Sorry for the miscommunication about that.

No console. I was deliberately trying to avoid the complexity of showing a console, then allowing for refresh and cancellation or whatever.

lmcmicu commented 3 years ago

Ok thanks. In that case the last commit should be enough to address this issue.