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.

Sign up for GitHub

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

Jump to bottom

Fix missing separator in module info line (usedby and using lists) #9241

Merged
merged 2 commits into from
Jul 19, 2021
Merged

Fix missing separator in module info line (usedby and using lists) #9241

merged 2 commits into from
Jul 19, 2021

Conversation

huangzhw
Copy link
Collaborator

genModulesInfoStringRenderModulesList lack |.

oranagra
oranagra previously approved these changes Jul 17, 2021
View reviewed changes
Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

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

Now the | sign that's added when the loop exits is excessive.
Also, maybe we wanna avoid adding the last | sign of the loop (when processing the tail)

@oranagra oranagra added state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten release-notes indication that this issue needs to be mentioned in the release notes labels Jul 17, 2021
@oranagra oranagra added this to To Do in 6.0 Backport via automation Jul 17, 2021
@oranagra oranagra added this to To Do in 6.2 Backport via automation Jul 17, 2021
@oranagra oranagra changed the title Fix module info genModulesInfoStringRenderModulesList lack separator Fix missing separator in module info line (usedby and using lists) Jul 19, 2021
@oranagra
Copy link
Member

@MeirShpilraien are you aware of anyone using it for more than one module?
do you think i should bother people by mentioning this fix in the release notes?

@MeirShpilraien
Copy link
Collaborator

@oranagra I am not aware of any module that is using it.

@oranagra oranagra removed the release-notes indication that this issue needs to be mentioned in the release notes label Jul 19, 2021
@oranagra
Copy link
Member

ok. arguably, for one module the previous code was ok, so i won't mention it.

@oranagra oranagra merged commit 1895e13 into redis:unstable Jul 19, 2021
@huangzhw huangzhw deleted the module_info branch July 19, 2021 10:35
oranagra pushed a commit to oranagra/redis that referenced this pull request Jul 20, 2021
@huangzhw @oranagra
Fix missing separator in module info line (usedby and using lists) (r…
05a4588 
…edis#9241)

Fix module info genModulesInfoStringRenderModulesList lack separator when there's more than one module in the list.

Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 1895e13)
oranagra pushed a commit to oranagra/redis that referenced this pull request Jul 21, 2021
@huangzhw @oranagra
Fix missing separator in module info line (usedby and using lists) (r…
70b621a 
…edis#9241)

Fix module info genModulesInfoStringRenderModulesList lack separator when there's more than one module in the list.

Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 1895e13)
oranagra pushed a commit that referenced this pull request Jul 21, 2021
@huangzhw @oranagra
Fix missing separator in module info line (usedby and using lists) (#…
2cd271e 
…9241)

Fix module info genModulesInfoStringRenderModulesList lack separator when there's more than one module in the list.

Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 1895e13)
oranagra pushed a commit that referenced this pull request Jul 21, 2021
@huangzhw @oranagra
Fix missing separator in module info line (usedby and using lists) (#…
6866117 
…9241)

Fix module info genModulesInfoStringRenderModulesList lack separator when there's more than one module in the list.

Co-authored-by: Oran Agra <oran@redislabs.com>
(cherry picked from commit 1895e13)
@oranagra oranagra moved this from To Do to In progress in 6.0 Backport Jul 22, 2021
@oranagra oranagra moved this from In progress to Done in 6.0 Backport Jul 22, 2021
@oranagra oranagra moved this from To Do to Done in 6.2 Backport Jul 22, 2021
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Sep 8, 2021
@huangzhw @oranagra
Fix missing separator in module info line (usedby and using lists) (r…
6358da7 
…edis#9241)

Fix module info genModulesInfoStringRenderModulesList lack separator when there's more than one module in the list.

Co-authored-by: Oran Agra <oran@redislabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oranagra oranagra oranagra approved these changes

@MeirShpilraien MeirShpilraien MeirShpilraien approved these changes

Assignees
No one assigned
Labels
state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten
Projects
6.0 Backport
Done
Redis 6.2 Backport
Status: Done
6.2 Backport
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@huangzhw @oranagra @MeirShpilraien