Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(scripts): [nan-973] slack integration for each environment #2168

Merged
merged 4 commits into from
May 17, 2024

Conversation

khaliqgant
Copy link
Member

Describe your changes

Allow a slack integration to be created at each env level. CLI fixes

Issue ticket number and link

NAN-973

Checklist before requesting a review (skip if just adding/editing APIs & templates)

  • I added tests, otherwise the reason is:
  • I added observability, otherwise the reason is:
  • I added analytics, otherwise the reason is:

Copy link

linear bot commented May 16, 2024

@@ -114,7 +114,7 @@ export function checkEnvVars(optionalHostport?: string) {

let pkgVersion: string | undefined = undefined;
export function getPkgVersion(debug = false) {
if (pkgVersion) {
if (pkgVersion && !debug) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Something that is very important to my workflow is to know where the CLI is running from. Because the pkgVersion is fetched in the getUserAgent() method on startup and cached the path wasn't being outputted. So with this the cache is skipped so the full debug information can be outputted if debug is set.

@khaliqgant khaliqgant requested review from bodinsamuel and TBonnin May 16, 2024 11:53
@@ -372,7 +372,7 @@ export const EnvironmentSettings: React.FC = () => {
};

const connectSlack = async () => {
const connectionId = `account-${accountUUID}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this retroactive somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. Going to manually update the prod connections. Only 20 or so

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe self hosted / enterprise are using this feature? if so could be done with a transaction

Copy link
Member Author

Choose a reason for hiding this comment

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

Enterprise definitely isn’t using the feature and since this is only for syncs / actions can almost guarantee that self hosted isn’t either

@khaliqgant khaliqgant merged commit 9051812 into master May 17, 2024
19 checks passed
@khaliqgant khaliqgant deleted the khaliq/nan-873-slack-integration-at-the-team-level branch May 17, 2024 07:02
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants