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

[APOLLO-1966]: diff configurations among clusters. issue #1966 #1996

Merged
merged 12 commits into from
Mar 3, 2019
Merged

[APOLLO-1966]: diff configurations among clusters. issue #1966 #1996

merged 12 commits into from
Mar 3, 2019

Conversation

kezhenxu94
Copy link
Member

@kezhenxu94 kezhenxu94 commented Feb 21, 2019

resolve #1966

@codecov-io
Copy link

codecov-io commented Feb 21, 2019

Codecov Report

Merging #1996 into master will decrease coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1996      +/-   ##
============================================
- Coverage     50.24%   50.17%   -0.08%     
+ Complexity     1980     1978       -2     
============================================
  Files           400      400              
  Lines         12349    12349              
  Branches       1275     1275              
============================================
- Hits           6205     6196       -9     
- Misses         5686     5694       +8     
- Partials        458      459       +1
Impacted Files Coverage Δ Complexity Δ
...work/apollo/biz/message/DatabaseMessageSender.java 57.14% <0%> (-10.21%) 6% <0%> (-2%)
...mework/apollo/portal/component/PortalSettings.java 65.07% <0%> (-4.77%) 5% <0%> (ø)
.../apollo/internals/RemoteConfigLongPollService.java 79.14% <0%> (-0.62%) 27% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dfa9f7a...df845d4. Read the comment docs.

@coveralls
Copy link

coveralls commented Feb 23, 2019

Coverage Status

Coverage decreased (-0.06%) to 53.891% when pulling df845d4 on kezhenxu94:feature/diff-among-clusters-1966 into dfa9f7a on ctripcorp:master.

@nobodyiam
Copy link
Member

Thanks! This feature looks nice, however, I think in the first step, we should automatically select the current cluster?

Suppose the app has 2 clusters, default and test, and if I clicked the compare configurations button in default cluster, then the default cluster should be selected automatically?

@kezhenxu94
Copy link
Member Author

Thanks! This feature looks nice, however, I think in the first step, we should automatically select the current cluster?

Suppose the app has 2 clusters, default and test, and if I clicked the compare configurations button in default cluster, then the default cluster should be selected automatically?

Reasonable and user-friendly, I've updated as suggested

@nobodyiam
Copy link
Member

I'm also wondering whether the comparison of comments is valueable in all situations?

Shall we provide an option like compare comments in the first step? I guess most of the time, what the users wants to compare may be the values instead of comments

@kezhenxu94
Copy link
Member Author

kezhenxu94 commented Mar 2, 2019

I'm also wondering whether the comparison of comments is valueable in all situations?

Shall we provide an option like compare comments in the first step? I guess most of the time, what the users wants to compare may be the values instead of comments

Can we just put a toggle button in the step 2 page and users can toggle (whether showing the diff of comments) without going back to page 1?

@nobodyiam
Copy link
Member

add a toggle is a good idea

@kezhenxu94
Copy link
Member Author

@nobodyiam Changes are made as discussed, now it works like this:
image

image

Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

LGTM

@nobodyiam nobodyiam merged commit 0b9d2aa into apolloconfig:master Mar 3, 2019
@kezhenxu94 kezhenxu94 deleted the feature/diff-among-clusters-1966 branch March 4, 2019 00:51
CrackerCat pushed a commit to CrackerCat/apollo-1 that referenced this pull request Jul 31, 2024
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.

有考虑做不同环境配置比较功能吗
4 participants