Skip to content

Conversation

@jimklimov
Copy link
Contributor

@basil : To facilitate eventual merge of the fixes done in PR #59 into PRs #75, #76 and #77 this branch replays those three PRs with proper merge conflict resolution to achieve the same codebase state as in PR #59 at this time, both synced with upstream/master as of today.

…ingOnNode() : log on which node we have a hit
…ingOnNode() : log the itemParams we compare to too
…check fail (some or all actual params not hit by filter)
…ingOnNode() : log the params arrays before and after doFilter()ing
…ingOnNode() : only filter executingUnitParams `if paramsToCompare.size() > 0` (do same as done for itemParams and avoid extra variable copying)
…ingOnNode() : refactor to fetch itemTaskName once
…ingOnNode() : refactor paramsToCompareSize to calculate it once
…UseForLimit by a comma, not just whitespace (which is not documented, but apparently nobody complained so used it instead)
…ToUseForLimit and paramsToCompare that are related to its usecase
…msToCompare() into a clean setter setParamsToCompare()
… values (repetitive/surrounding separator chars)
…se_paramsToUseForLimit() : honor the ordering of a List type
… testThrottleJob_constructor_should_parse_paramsToUseForLimit()
…uld_parse_paramsToUseForLimit() to look at truncation of whitespace/separators in input
… get) in testThrottleJob_constructor_should_parse_paramsToUseForLimit()
…ARATOR until it is introduced"

This reverts commit 1219b1e
since the token is introduced in this re-merged codebase.
@mamh2021
Copy link

I'm confused why this paramsToUseForLimit variable was splited by different char at line 228 and line 101 in file ThrottleJobProperty.java

when this PR will be merged???

@jimklimov
Copy link
Contributor Author

jimklimov commented Dec 18, 2020

I wonder the same... @basil? Any blockers for this PR (or rather for related sub-PRs #77 and #76 that you wanted split out), or just collecting dust?

@basil
Copy link
Member

basil commented Dec 21, 2020

Thanks for the reminder, I’ll try to process this in the next week or so.

@jimklimov
Copy link
Contributor Author

Followed up by #103 and #104, released in 2.1.

@jimklimov jimklimov closed this Dec 27, 2020
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.

3 participants