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

Fix #17869 - Removing the stale table header cell. #17893

Closed
wants to merge 1 commit into from

Conversation

vimalMK
Copy link
Contributor

@vimalMK vimalMK commented Nov 13, 2022

Signed-off-by: Vimal K vimalinfo10@gmail.com

Description

Removing the stale table header cell.

image

Fixes #17869

Before submitting pull request, please review the following checklist:

  • Make sure you have read our CONTRIBUTING.md document.
  • Make sure you are making a pull request against the correct branch. For example, for bug fixes in a released version use the corresponding QA branch and for new features use the master branch. If you have a doubt, you can ask as a comment in the bug report or on the mailing list.
  • Every commit has proper Signed-off-by line as described in our DCO. This ensures that the work you're submitting is your own creation.
  • Every commit has a descriptive commit message.
  • Every commit is needed on its own, if you have just minor fixes to previous commits, you can squash them.
  • Any new functionality is covered by tests.

Signed-off-by: Vimal K <vimalinfo10@gmail.com>
@williamdes williamdes added this to the 5.2.1 milestone Nov 13, 2022
Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

I am looking into this but I feel there was a reason. So for now I am blocking this until I figure out

Sources:
76409e2#diff-401fa9ac455f80ecd82d75155b189c586eadf55f976b2050915d031eaf79a39dR2050-R2065
37d50c1#diff-98f45ff6ce5760b40756ed7ce67dc0b880e8ba55f1298d7c79bd7bb8b49a3d60R671-R685

It goes back 20 years ago ...
What was the reason 🤔

@williamdes
Copy link
Member

williamdes commented Nov 14, 2022

Seems like it goes back 21 years ago from 16843c6 (16843c6#diff-7b9a476426ac829a8aacf9901ba4cd3d0e6f75fe067defa00be7e5c258f9c6f7R662)

Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

This fix needs to also delete the line in

. '>' . $fullOrPartialTextLink . '</th>';

You need to go to information_schema DB and OPTIMIZER_TRACE view
See: #17869 (comment)

@williamdes williamdes removed this from the 5.2.1 milestone Nov 14, 2022
@vimalMK
Copy link
Contributor Author

vimalMK commented Nov 14, 2022

This fix needs to also delete the line in

. '>' . $fullOrPartialTextLink . '</th>';

You need to go to information_schema DB and OPTIMIZER_TRACE view See: #17869 (comment)

shall I go ahead and remove it now or wait for the update regarding the purpose?

@williamdes
Copy link
Member

This fix needs to also delete the line in

. '>' . $fullOrPartialTextLink . '</th>';

You need to go to information_schema DB and OPTIMIZER_TRACE view See: #17869 (comment)

shall I go ahead and remove it now or wait for the update regarding the purpose?

I think you can go ahead, please be sure to check as much as possible that it is not breaking something and remove the method param if it is not used anymore

@vimalMK
Copy link
Contributor Author

vimalMK commented Nov 14, 2022

This fix needs to also delete the line in

. '>' . $fullOrPartialTextLink . '</th>';

You need to go to information_schema DB and OPTIMIZER_TRACE view See: #17869 (comment)

shall I go ahead and remove it now or wait for the update regarding the purpose?

I think you can go ahead, please be sure to check as much as possible that it is not breaking something and remove the method param if it is not used anymore

Sounds good.

Copy link
Member

@MauricioFauth MauricioFauth left a comment

Choose a reason for hiding this comment

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

I don't think such change should go to a QA branch.

@williamdes
Copy link
Member

I don't think such change should go to a QA branch.

Yeah, it's a bit too risky. Let's merge into master branch

@williamdes williamdes changed the base branch from QA_5_2 to master December 1, 2022 23:33
@williamdes williamdes linked an issue Dec 1, 2022 that may be closed by this pull request
@williamdes
Copy link
Member

This has to be re-done, the code has changed too much for me to be able to merge this

@williamdes williamdes closed this Apr 27, 2024
@williamdes williamdes self-assigned this Apr 27, 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.

Extra table header cell incorrectly added
3 participants