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

feat: Save PR message if creation fails #113

Merged

Conversation

twavv
Copy link
Contributor

@twavv twavv commented May 15, 2023

Saves the pull request description (entered via editor) to a temporary file if creating the pull request fails. It will re-use the title/description if the user tries to re-create the PR (also just logs the file path to the console in case they want that).

The most common cause of this error (in my experience) is expired auth tokens and/or the base branch of a stack having been deleted on GitHub. (we should provide a better experience for the latter but that's outside the scope of this PR)

@aviator-app
Copy link
Contributor

aviator-app bot commented May 15, 2023

Current Aviator status

Aviator will automatically update this comment as the status of the PR changes.
Comment /aviator refresh to force Aviator to re-examine your PR (or learn about other /aviator commands).

This PR was merged using Aviator.


See the real-time status of this PR on the Aviator webapp.

// We need to open an editor to ask the user. Try to populate with
// reasonable defaults here.
// If a saved pull request description exists, use that.
saveFile := filepath.Join(os.TempDir(), fmt.Sprintf("av-pr-%s.md", sanitize.FileName(opts.BranchName)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use avDir

avDir := path.Join(repo.GitDir(), "av")
instead of the TempDir. tmpfs is ephemeral.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think tempdir makes sense here. It's decently ephemeral data and it's also possible that if they fail to create a PR using av they'll just create it on GH and we'll never clean up that old data. It's not liable to be more than a few kb for most people but still I think tempfs makes sense here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ephemeral here means it won't survive after reboot. When a machine has a network issue and has to reboot, after reboot the message is lost.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think that's a fine trade off to make here. Most people don't reboot very often and it's likely (at least I imagine) that if you fail to open a PR you'll deal with it before the next time you'd reboot your machine.

@aviator-app aviator-app bot merged commit f5b183a into master May 15, 2023
@aviator-app aviator-app bot deleted the travis/mer-2207-save-pr-message-if-pr-creation-fails branch May 15, 2023 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants