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: add grid object to all Formatter calls #558

Merged
merged 2 commits into from Nov 29, 2020

Conversation

ghiscoding
Copy link
Collaborator

No description provided.

@ghiscoding
Copy link
Collaborator Author

ghiscoding commented Nov 27, 2020

@6pac
After doing these changes, my VSCode told me that the function callFormatter() is not used anywhere in the file and after doing a full project search, I see it's not used in the entire lib and it isn't a public function either, see it on this line.

Was that code something you added while working on the column auto-size of the width?
Perhaps it's a function that is no longer necessary and should be removed? I don't see any references anywhere so... should I delete it in this PR?

@ghiscoding ghiscoding requested a review from 6pac November 27, 2020 16:09
@6pac
Copy link
Owner

6pac commented Nov 28, 2020

I have no idea where callFormatter comes from. It's not in any of my old repos, but I downloaded the current master back to Jun 2019 and it's still in there. I suspect someone added it at some point in a PR.
Perhaps we should leave it for future investigation, it's not hurting anything. I don't object if you'd prefer to delete it, though.

@ghiscoding
Copy link
Collaborator Author

It actually comes from my commit 🤣
It was copied over when I've starting merging the frozen feature from X-SlickGrid but there's few pieces that weren't used.
So I'm removing it since it was used in X-SlickGrid but not in our version, so it's safe to remove.

PR is ready

image

@6pac 6pac merged commit 8d957f1 into master Nov 29, 2020
@ghiscoding ghiscoding deleted the bugfix/formatter-always-include-grid-object branch November 29, 2020 03:12
@ghiscoding
Copy link
Collaborator Author

ghiscoding commented Dec 1, 2020

@6pac
I'm pretty much done with PRs for a while, would you mind doing another release with all PRs I've done recently.
Actually I got another PR #561 in the queue lol

Thanks Mate

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.

None yet

2 participants