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: added Copy Web Console URL action #306

Merged
merged 86 commits into from Aug 5, 2022
Merged

feat: added Copy Web Console URL action #306

merged 86 commits into from Aug 5, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jul 27, 2022

Changelog

Added "Copy Web Console URL" action in session's copy-context menu.

Bugfixes

Enhancements

Closes #296.

Notes

Is this needed also in CLI?
No new tests as openWebConsole - generates a valid aws url to log covers it.

Any comments, enhancements, proposals, fixes are very welcome.

@pethron
Copy link
Contributor

pethron commented Jul 27, 2022

Would be awesome to open the browser from the CLI either. In general we're trying to keep the correspondence of actions between the gui and cli.

@ghost
Copy link
Author

ghost commented Jul 27, 2022

Agree. I'll have a check on CLI also.

@ghost
Copy link
Author

ghost commented Jul 27, 2022

Now that I got into it, it was easy to implement. But now it wonders if CLI command session open-web-console should be changed into session web-console open|print. I made a question about it in original issue: #296 (comment)

@pethron
Copy link
Contributor

pethron commented Jul 27, 2022

I think the two separate commands open | print would be better, as you suggested in the original issue.

@ghost
Copy link
Author

ghost commented Jul 27, 2022

Great! I'll create documentation and tests for it and test it also in linux and mac tomorrow before pushing the CLI changes.

@ghost
Copy link
Author

ghost commented Jul 29, 2022

Added CLI action and Jest-tests for it.
Ran core, cli and desktop-app tests and all succeeded.
Tested with AWS SSO in Windows 10, Ubuntu 22.04 and macOS 12.4 Monterey.
I feel this is ready for a review.

@pethron
Copy link
Contributor

pethron commented Jul 29, 2022

Great work, Sami! 🥇 I think @ericvilla can review this next week, and we can integrate everything in the next release

@rick-rtt
Copy link
Contributor

rick-rtt commented Aug 5, 2022

Thanks for the excellent pull request! After review, we decided to add an optional flag --print to the leapp session open-web-console command instead of using a different command, everything else is perfect as it is! We are going to merge this to master and release it later today, so stay tuned! And thanks again!

@ghost
Copy link
Author

ghost commented Aug 5, 2022

Glad to hear. It was 50/50 whether to use flag or create a new command and it is easy to change.

rick-rtt pushed a commit that referenced this pull request Aug 5, 2022
@rick-rtt rick-rtt merged commit fa30c07 into Noovolari:master Aug 5, 2022
@ghost ghost deleted the 296-copy-web-console-url-action branch August 23, 2022 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow copying the URL for "Open Web Console"
5 participants