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.

Sign up for GitHub

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

Jump to bottom

WIP: Issue/492 empty tables #725

Merged
merged 7 commits into from
Jul 30, 2024
Merged

WIP: Issue/492 empty tables #725

merged 7 commits into from
Jul 30, 2024

Conversation

AlanFCMV
Copy link
Member

@AlanFCMV AlanFCMV commented Dec 3, 2021 •
edited
Loading

Closes #492

Waiting on a new phpally version update.
Needs a ufixit form that allows you to delete the table.

@AlanFCMV AlanFCMV self-assigned this Dec 3, 2021
@AlanFCMV AlanFCMV linked an issue Dec 3, 2021 that may be closed by this pull request
Add new check for empty tables #492
Open
@rob-3 rob-3 mentioned this pull request Jan 19, 2022
Merged
@bagofarms bagofarms assigned SimonRothUCF and unassigned AlanFCMV Feb 3, 2022
@rob-3 rob-3 added this to the 3.3.0 milestone Feb 8, 2022
@rob-3 rob-3 changed the base branch from dev/v3-2-0 to dev/v3-3-0 February 8, 2022 19:51
@SimonRothUCF SimonRothUCF removed their assignment Mar 17, 2022
@bagofarms
Copy link
Member

I'm thinking we can go ahead and merge this so the feature gets out there, then we can create a new issue for the UFIXIT.

@rob-3 rob-3 self-assigned this Apr 14, 2022
@SimonRothUCF SimonRothUCF assigned SimonRothUCF and unassigned rob-3 Apr 14, 2022
@SimonRothUCF SimonRothUCF requested a review from rob-3 April 14, 2022 19:45
@rob-3
Copy link
Contributor

rob-3 commented Apr 25, 2022

I'm thinking we can go ahead and merge this so the feature gets out there, then we can create a new issue for the UFIXIT.

I am happy to merge this; as it doesn't appear to break anything, but I also can't get the event to fire. The logs don't show it as even recognized by UDOIT, and my efforts to write a form for it don't appear to do anything. I recommend keeping this open until someone figures out how to write a form for it.

@SimonRothUCF SimonRothUCF removed their assignment Apr 27, 2022
@bagofarms
Copy link
Member

Updated code with latest 3.3.0, but I am also unable to get the error to trigger. I'm going to push this off for a future release.

@bagofarms bagofarms removed this from the 3.3.0 milestone Aug 5, 2022
@bagofarms
Copy link
Member

I was able to get it to trigger!
Screen Shot 2022-12-16 at 4 57 58 PM

@bagofarms bagofarms requested review from dmols and removed request for rob-3 December 16, 2022 21:59
dmols
dmols approved these changes Dec 16, 2022
View reviewed changes
Copy link
Contributor

@dmols dmols left a comment

Choose a reason for hiding this comment

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

Empty table warning activates on my end as well! All other checks work as intended. Clear to push!

@bagofarms bagofarms changed the base branch from dev/v3-3-0 to main December 19, 2022 19:32
@dmols
Copy link
Contributor

dmols commented Jun 20, 2023

I'm unable to replicate the solution this time around. Could you take a look when you get a chance, @taheralfayad? Let me know if you see something similar to what Jacob posted above.

@dmols dmols requested a review from taheralfayad June 29, 2023 16:18
@dmols dmols self-requested a review July 3, 2023 19:25
@dmols dmols self-assigned this May 21, 2024
@dmols
Copy link
Contributor

dmols commented May 21, 2024

Was able to trigger this again. Will test to see if it's good for merge. I'll open an issue to implement a UFIXIT form to go along with the rule.

@taheralfayad taheralfayad changed the base branch from main to dev July 29, 2024 13:48
taheralfayad
taheralfayad approved these changes Jul 30, 2024
View reviewed changes
Copy link
Contributor

@taheralfayad taheralfayad left a comment

Choose a reason for hiding this comment

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

LGTM

@dmols dmols merged commit 908c934 into dev Jul 30, 2024
2 checks passed
dmols
dmols reviewed Jul 30, 2024
View reviewed changes
Copy link
Contributor

@dmols dmols left a comment

Choose a reason for hiding this comment

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

Changes looked good; merged to dev for now! Will be adding an issue to tackle the implementation of a UFIXIT form in the future. Great work!

@dmols dmols mentioned this pull request Jul 30, 2024
Open
@dmols dmols deleted the issue/492-empty-tables branch November 12, 2024 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmols dmols dmols approved these changes

@taheralfayad taheralfayad taheralfayad approved these changes

Assignees

@dmols dmols

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add new check for empty tables
6 participants
@AlanFCMV @bagofarms @rob-3 @dmols @taheralfayad @SimonRothUCF