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

Remove legacy "autoReadPattern" slack. #9761

Merged
merged 2 commits into from
Jan 6, 2025
Merged

Conversation

flvndvd
Copy link
Contributor

@flvndvd flvndvd commented Jan 6, 2025

Description

Follow up of the recent work around adding support for spaces in Slack auto read patterns. This PR removes the deprecated code.

Risk

Deploy Plan

  • Deploy
  • Run the migration to delete deprecated column.

Copy link
Contributor

devloai bot commented Jan 6, 2025

Automatic code review is enabled for this repository. I will perform a review of this PR.

@flvndvd flvndvd added the migration-ack 📁 Label to acknowledge that a migration is required. label Jan 6, 2025
@flvndvd flvndvd marked this pull request as ready for review January 6, 2025 08:03
Copy link

github-actions bot commented Jan 6, 2025

Warnings
⚠️

Files in **/lib/models/ have been modified and the PR has the migration-ack label. Don't forget to run the migration from prodbox.

Generated by 🚫 dangerJS against 969d8a2

Copy link
Contributor

@devloai devloai bot left a comment

Choose a reason for hiding this comment

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

The changes look good and systematically remove the deprecated autoReadChannelPattern functionality across all layers. I've suggested a minor improvement to document the migration file better for future reference. Otherwise, the removal is clean with no remaining references to the old field.

@@ -0,0 +1,2 @@
-- Migration created on Jan 06, 2025
ALTER TABLE slack_configurations DROP COLUMN "autoReadChannelPattern";
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding a comment explaining that this column was deprecated in favor of autoReadChannelPatterns for future reference. Something like:

Suggested change
ALTER TABLE slack_configurations DROP COLUMN "autoReadChannelPattern";
-- Migration created on Jan 06, 2025
-- Drop legacy autoReadChannelPattern column which was deprecated in favor of autoReadChannelPatterns
ALTER TABLE slack_configurations DROP COLUMN "autoReadChannelPattern";

Copy link
Contributor

@JulesBelveze JulesBelveze left a comment

Choose a reason for hiding this comment

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

LGTM

@flvndvd flvndvd merged commit 7104095 into main Jan 6, 2025
5 checks passed
@flvndvd flvndvd deleted the flav/cleanup-slack-patterns branch January 6, 2025 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migration-ack 📁 Label to acknowledge that a migration is required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants