Skip to content

Print server banner in store and diff CLI commands - #5026

Open
amina825 wants to merge 6 commits into
Ericsson:masterfrom
amina825:print-banner-in-store-and-diff
Open

Print server banner in store and diff CLI commands#5026
amina825 wants to merge 6 commits into
Ericsson:masterfrom
amina825:print-banner-in-store-and-diff

Conversation

@amina825

Copy link
Copy Markdown
Contributor

Issue #1861 - The server announcement banner was already printed in the login command. This change extends the same behaviour to the store and diff commands (when a remote server is involved).

  • Extract print_banner() helper in cmd_line_client.py to avoid duplicating the get/decode/log logic
  • Call print_banner() in store.main() after setup_client()
  • Call print_banner() in handle_diff_results() after setup_client(), outside the try/except so network errors are not silently swallowed
  • Extend test_announcement_showing_in_cli to cover store and diff

Issue Ericsson#1861 - The server announcement banner was already printed in
the login command. This change extends the same behaviour to the
store and diff commands (when a remote server is involved).

- Extract print_banner() helper in cmd_line_client.py to avoid
  duplicating the get/decode/log logic
- Call print_banner() in store.main() after setup_client()
- Call print_banner() in handle_diff_results() after setup_client(),
  outside the try/except so network errors are not silently swallowed
- Extend test_announcement_showing_in_cli to cover store and diff
@amina825
amina825 force-pushed the print-banner-in-store-and-diff branch from 6e798fc to 6baa10a Compare August 14, 2026 20:50

@bruntib bruntib left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you, it's a nice enhancement.

However, I have some suggestions for the implementation:

get_announcement_msg() is called only from print_banner(). Couldn't these two functions be merged?
Also, split_server_url() could also be called from print_banner(), because it's called either way at every occurrence. This way we can save the protocol, host and port parameters. The args.server_url should be enough.
Could we print the log message to LOG? I think, the log=None parameter could be eliminated.

Finally, it would be useful if this banner text would be printed right at the very beginning of each command. The problem is that it gets lost among the lot of other log messages. If you look at handle_login(), the print_banner() call is at the very beginning, but in case of handle_diff_results() it's quite low. Maybe, this could be the first function call after init_logger() in every case.

Please, look at the help page of Codechecker cmd --help. Here you can see, that right now only CodeChecker cmd login and CodeChecker cmd diff are covered. But, there are many other sub-commands, too. A handle_<blabla> function belongs to each. These are also candidates for printing the banner.

Thank you!

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.

2 participants