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

fix: bug finding on ui/ux and docker built image issue #81

Merged

Conversation

zulkhair
Copy link
Collaborator

@zulkhair zulkhair commented Oct 17, 2024

Closes: WORLD-1196, WORLD-1205, WORLD-1206, WORLD-1207

Overview

Fix bug findings on world cli v1.3.1-Beta4

Brief Changelog

  • Fix docker image not builded without exec world cardinal purge firs. it's because container need to be removed first so that docker create new container with new image
  • Remove red and yellow color for container name
  • Fix --log-level flag not functioned properly, it's because the envar is not passed to the docker env.
  • Move -v and -c from global flags to only flag in 3rd subcommand
  • Add help for --log-level value (ex : info, debug, etc)

Testing and Verifying

  • Manually tested
  • Still covered by unit test

Copy link

graphite-app bot commented Oct 17, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “graphite/merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Collaborator Author

zulkhair commented Oct 17, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @zulkhair and the rest of your teammates on Graphite Graphite

@zulkhair zulkhair changed the title fix bug finding on ui/ux and docker built image issue fix: bug finding on ui/ux and docker built image issue Oct 17, 2024
@zulkhair zulkhair marked this pull request as ready for review October 17, 2024 02:05
@zulkhair zulkhair force-pushed the daim/fix_bug_finding_on_ui_ux_and_docker_built_image_issue branch 2 times, most recently from e4eb53c to 338cc4b Compare October 17, 2024 03:59
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 73.63636% with 29 lines in your changes missing coverage. Please review.

Project coverage is 47.71%. Comparing base (e1adc44) to head (433d967).

Files with missing lines Patch % Lines
common/docker/client_image.go 0.00% 15 Missing ⚠️
common/docker/service/evm.go 38.09% 9 Missing and 4 partials ⚠️
common/docker/client_container.go 94.73% 1 Missing ⚠️
Additional details and impacted files
@@                     Coverage Diff                     @@
##           daim/fix_sentry_version      #81      +/-   ##
===========================================================
+ Coverage                    45.72%   47.71%   +1.98%     
===========================================================
  Files                           56       56              
  Lines                         3427     3521      +94     
===========================================================
+ Hits                          1567     1680     +113     
+ Misses                        1655     1627      -28     
- Partials                       205      214       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@rmrt1n rmrt1n left a comment

Choose a reason for hiding this comment

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

Tested locally. The images are being rebuilt now. Other than some nits, everything else looks good!

common/config/config.go Outdated Show resolved Hide resolved
common/docker/client_container.go Show resolved Hide resolved
cmd/world/cardinal/start.go Show resolved Hide resolved
@zulkhair zulkhair force-pushed the daim/fix_bug_finding_on_ui_ux_and_docker_built_image_issue branch 2 times, most recently from 5b3aaf4 to 7cc3ea5 Compare October 18, 2024 17:23
@zulkhair zulkhair requested a review from rmrt1n October 21, 2024 02:45
@zulkhair zulkhair force-pushed the daim/fix_bug_finding_on_ui_ux_and_docker_built_image_issue branch from 7cc3ea5 to 433d967 Compare October 21, 2024 12:13
Copy link

graphite-app bot commented Oct 22, 2024

Merge activity

Closes: WORLD-1196, WORLD-1205, WORLD-1206, WORLD-1207

## Overview

Fix bug findings on world cli v1.3.1-Beta4

## Brief Changelog

- Fix docker image not builded without exec `world cardinal purge` firs. it's because container need to be removed first so that docker create new container with new image
- Remove red and yellow color for container name
- Fix `--log-level` flag not functioned properly, it's because the envar is not passed to the docker env.
- Move `-v` and `-c` from global flags to only flag in 3rd subcommand
- Add help for `--log-level` value (ex : info, debug, etc)

## Testing and Verifying

- Manually tested
- Still covered by unit test
@zulkhair zulkhair force-pushed the daim/fix_bug_finding_on_ui_ux_and_docker_built_image_issue branch from 433d967 to 84f5fb5 Compare October 22, 2024 09:25
@zulkhair zulkhair changed the base branch from daim/fix_sentry_version to main October 22, 2024 09:28
@graphite-app graphite-app bot merged commit 84f5fb5 into main Oct 22, 2024
5 of 7 checks passed
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