Skip to content

fix(admin): correct the count field of plugin-metadata/global-rule#6155

Merged
spacewander merged 8 commits into
apache:masterfrom
mangoGoForward:fix/unfiy-count-type
Jan 23, 2022
Merged

fix(admin): correct the count field of plugin-metadata/global-rule#6155
spacewander merged 8 commits into
apache:masterfrom
mangoGoForward:fix/unfiy-count-type

Conversation

@mangoGoForward

@mangoGoForward mangoGoForward commented Jan 19, 2022

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Correct the count field of plugin-metadata. please see comment

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@mangoGoForward
mangoGoForward marked this pull request as draft January 19, 2022 15:40
@mangoGoForward
mangoGoForward marked this pull request as ready for review January 19, 2022 15:48
spacewander
spacewander reviewed Jan 20, 2022
View reviewed changes

@spacewander spacewander left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you also solve the global_rules in this PR?

@mangoGoForward

mangoGoForward commented Jan 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Could you also solve the global_rules in this PR?

Ok, I will be commit later.

@mangoGoForward

mangoGoForward commented Jan 20, 2022

Copy link
Copy Markdown
Contributor Author

Could you also solve the global_rules in this PR?

Already done. please see 7a2ce71.

tokers
tokers reviewed Jan 20, 2022
View reviewed changes
Comment thread t/admin/global-rules2.t

__DATA__

=== TEST 1: list empty resources

@tokers tokers Jan 20, 2022

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.

Can we add some test cases for non-empty resources?

@mangoGoForward mangoGoForward Jan 20, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done. please see b94a86c.

spacewander
spacewander reviewed Jan 21, 2022
View reviewed changes
Comment thread t/admin/global-rules2.t Outdated
}
--- response_body
{"action":"set","node":{"key":"/apisix/global_rules/1","value":{"id":"1","plugins":{"proxy-rewrite":{"uri":"/"}}}}}
--- request

@spacewander spacewander Jan 21, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is no need to repeat the two sections below?

@mangoGoForward mangoGoForward Jan 21, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I want to add a test cases for non-empty resources like TEST 3, so put a new global rules.

@spacewander spacewander Jan 21, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But you already set them (--- request, --- no_error_log) in https://github.com/apache/apisix/pull/6155/files#diff-150a3eabdca95c99b2f057a9981111fd3dc0f7c830999549b60ce791265319c3R28

@mangoGoForward mangoGoForward Jan 21, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Got it, I have removed request and error log phrase.

@spacewander spacewander changed the title fix(admin): correct the count field of plugin-metadata fix(admin): correct the count field of plugin-metadata/global-rule Jan 21, 2022
@spacewander
spacewander requested a review from tokers January 21, 2022 09:22
@spacewander
spacewander merged commit a813b5a into apache:master Jan 23, 2022
@mangoGoForward
mangoGoForward deleted the fix/unfiy-count-type branch January 24, 2022 01:11
@spacewander spacewander mentioned this pull request Feb 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@spacewander spacewander spacewander approved these changes
@starsz starsz starsz approved these changes
@tzssangglass tzssangglass tzssangglass approved these changes
@tokers tokers Awaiting requested review from tokers

Assignees

No one assigned

Labels

need backport

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@mangoGoForward @spacewander @tokers @starsz @tzssangglass