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

Feature/demrum 440 dsyms list command #61

Merged

Conversation

carlosmcevilly
Copy link
Contributor

Add command to request list of dSYMs from the cloud API.

@carlosmcevilly carlosmcevilly requested review from a team as code owners December 24, 2024 00:20
debug?: boolean;
}

iOSCommand
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this block needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which block? I'm seeing code that doesn't line up with a block above your comments, maybe the branch got out of sync with what's shown here(?).

}
});

iOSCommand.parse(process.argv);
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think this should be here. If you look in index.ts file, it has the program defined, adds the different commands and parses

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed; thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

These look similar to whats in androidInputValidations. Perhaps we could consolidate it and make them generically useable for both android and iOS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I've added this update to the previous PR so once that is merged and I rebase this branch then we will see the change here as well.

@carlosmcevilly carlosmcevilly force-pushed the feature/DEMRUM-440-dsyms-list-command branch from 50d7924 to c4e0895 Compare January 10, 2025 23:30
iOSCommand
.command('upload')
.showHelpAfterError(true)
.usage('--file <path>')
.description(iOSUploadDescription)
.summary('Upload a dSYMs .zip file to the symbolication service')
.requiredOption('--file <path>', 'Path to the dSYMs .zip file')
.option('--debug', 'Enable debug logs')
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intentional to remove --debug option from Upload right now? If so, you'd want to keep UploadiOSOptions in-sync

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I think I want to keep that actually.

@carlosmcevilly carlosmcevilly merged commit 49ab8cc into signalfx:main Jan 11, 2025
2 checks passed
@carlosmcevilly carlosmcevilly deleted the feature/DEMRUM-440-dsyms-list-command branch January 11, 2025 00:58
@github-actions github-actions bot locked and limited conversation to collaborators Jan 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants