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

Export timeline log feature #1533

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

denis-remitly
Copy link

Please verify the following:

  • yarn build-and-test:local passes
  • I have added tests for any new features, if relevant
  • README.md (or relevant documentation) has been updated with your changes

Describe your PR

Screenshot 2025-01-23 at 12 24 52

This PR adds the ability to export the timeline log into a JSON file.

Sorry, something went wrong.

denis-remitly and others added 5 commits January 23, 2025 12:03

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@jamonholmgren
Copy link
Member

Very cool feature!

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@coolsoftwaretyler coolsoftwaretyler self-requested a review March 17, 2025 15:22
Copy link
Contributor

@coolsoftwaretyler coolsoftwaretyler left a comment

Choose a reason for hiding this comment

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

Hey @denis-remitly - thanks for this! I really like it a lot. I have two suggestions. I'd love if you can tackle both in this PR before we merge, but only one of them is really blocking (I think an empty timeline should be an empty array, rather than nothing at all).

)
console.log(`Exported timeline log to ${downloadDir}`)
} else {
console.error("There is nothing to export.")
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: @denis-remitly - what do you think about still exporting an empty array here instead? That would be my preference.

function downloadLog() {
if (commands.length > 0) {
const homeDir = os.homedir()
const downloadDir = path.join(homeDir, "Downloads")
Copy link
Contributor

Choose a reason for hiding this comment

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

issue: if ~/Downloads doesn't exist, this fails silently. Can we either give the user an alert, or perhaps prompt them to set a download location instead of just assuming this path will exist for them?

I don't think we need to block on this because it doesn't crash the app, but if we can add that to this PR or quickly follow up, that would be great.

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.

None yet

3 participants