Skip to content

Conversation

@chrismckiernan
Copy link
Contributor

@chrismckiernan chrismckiernan commented May 31, 2024

@chrismckiernan chrismckiernan changed the title 1948 - Send email when PDL/Supervisor is added/updated to a member pr… 1948 - Send email when PDL/Supervisor is added/updated to a member profile May 31, 2024
@chrismckiernan chrismckiernan force-pushed the feature-1948/send-pdl--notification-email branch 7 times, most recently from a865933 to b08448e Compare May 31, 2024 12:54
@chrismckiernan chrismckiernan force-pushed the feature-1948/send-pdl--notification-email branch from b08448e to 4ae04f0 Compare May 31, 2024 12:57
@chrismckiernan chrismckiernan force-pushed the feature-1948/send-pdl--notification-email branch 2 times, most recently from 7aa1ef8 to b971d54 Compare May 31, 2024 14:25
…t getting required input for sending the emails
@chrismckiernan chrismckiernan force-pushed the feature-1948/send-pdl--notification-email branch from b971d54 to e4bbd76 Compare May 31, 2024 14:25
Copy link
Contributor

@vhscom vhscom left a comment

Choose a reason for hiding this comment

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

In general I'd prefer to see us using const over let unless we plan to mutate the assignment later in the control flow logic. I think there were some examples of let in this code which were followed as a pattern, but it wasn't the ideal pattern.

@chrismckiernan chrismckiernan force-pushed the feature-1948/send-pdl--notification-email branch from e8b42be to 2d62944 Compare May 31, 2024 16:03
@vhscom vhscom self-requested a review May 31, 2024 16:08
Copy link
Contributor

@vhscom vhscom left a comment

Choose a reason for hiding this comment

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

LGTM

@chrismckiernan chrismckiernan merged commit 41a626a into develop May 31, 2024
@chrismckiernan chrismckiernan deleted the feature-1948/send-pdl--notification-email branch June 3, 2024 21:57
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.

4 participants