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

Improving Describe-TQ Command Description #684

Merged

Conversation

Shivs11
Copy link
Member

@Shivs11 Shivs11 commented Oct 1, 2024

What was changed

Why?

  • opened a new PR since the repo went through some changes with respect to generating code (now uses a .yml file)
  • attempted at resolving conflicts on the previous open PR but was faced with unknown issues
  • thought it was best, in the interest of time and given the relatively small size of the PR, to open a new branch and close the issue.

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

Comment on lines +1515 to +1517
Such tasks are not counted by these metrics. Despite the inaccuracy of
these two metrics, the derived metric of `BacklogIncreaseRate` is accurate
for backlogs older than a few seconds.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to contradict the statement above re. the Backlog Increase Rate:

This is equivalent to:
`TasksAddRate` - `TasksDispatchRate`.

(Also, should it be BacklogIncreaseRate, not Backlog Increase Rate?)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think what that statement means to say is that: Despite the inaccuracies put forward by the two metrics (wrt not counting eagerly dispatched tasks, the BacklogIncreaseRate metric is an accurate estimate for the number of tasks that are actually present in the backlog.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This may be pedantic but let's change that text to then read: "This is roughly equivalent to"—if we say "This is equivalent to", someone will probably expect to be able to do math and get the same result, and it sounds like that's not the case.

Also, still same question about the spaces in Backlog Increase Rate—can you also change that while you're here if it's supposed to be BacklogIncreaseRate?

Copy link
Member Author

@Shivs11 Shivs11 Oct 2, 2024

Choose a reason for hiding this comment

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

Sorry, I forgot to address that in my response but had made the change locally. Yes, I shall be changing Backlog Increase Rate to the required output.

Made the changes required as well; thank you for taking a peek at this!

@Shivs11 Shivs11 requested a review from josh-berry October 1, 2024 15:11
Copy link
Collaborator

@josh-berry josh-berry left a comment

Choose a reason for hiding this comment

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

Thanks for being patient with my nitpicking. :)

@Shivs11 Shivs11 merged commit 4daeb0e into main Oct 2, 2024
7 checks passed
@Shivs11 Shivs11 deleted the shivam/improve-describe-tq-command-description-updated branch October 2, 2024 19:48
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