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

changed notification.config from varchar to text. #3763

Merged
merged 1 commit into from
Sep 21, 2023
Merged

changed notification.config from varchar to text. #3763

merged 1 commit into from
Sep 21, 2023

Conversation

FJBlok
Copy link
Contributor

@FJBlok FJBlok commented Sep 19, 2023

Tick the checkbox if you understand [x]:

  • I have read and understand the pull request rules.

Description

I've changed the way a monitor config is stored in the database.
This was a varchar and now it is a text record. This will increase the length of the record you can store in that field.
I ran into an issue with ntfy(push notification service) that the config json was to big for that field.
Also this was marked as TODO in the code.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
    (including JSDoc for methods)
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

@louislam louislam added this to the 2.0.0 milestone Sep 19, 2023
@louislam
Copy link
Owner

louislam commented Sep 19, 2023

Thanks, almost forget it. It is for 2.0.0.
Will add another pr for 1.23.3 maybe, since 1.23.x is using sql files to patch the database.

@louislam louislam merged commit 2266f31 into louislam:master Sep 21, 2023
14 checks passed
louislam added a commit that referenced this pull request Sep 21, 2023
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