Page MenuHomePhabricator
Differential D6532

[Nix] Only prompt for direnv usage on install
ClosedPublic
Actions

Authored by • jon on Feb 2 2023, 9:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 23, 1:36 PM
Unknown Object (File)
Feb 23 2025, 1:15 AM
Unknown Object (File)
Feb 20 2025, 10:34 AM
Unknown Object (File)
Feb 16 2025, 7:54 AM
Unknown Object (File)
Feb 16 2025, 7:54 AM
Unknown Object (File)
Feb 16 2025, 7:54 AM
Unknown Object (File)
Feb 16 2025, 7:48 AM
Unknown Object (File)
Jan 13 2025, 11:47 PM
View All 69 Files
Subscribers
tomek

Details

Reviewers
ashoat
varun
atul
bartek
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Commits
rCOMMaa565352bba0: [Nix] Only prompt for direnv usage on install
Summary

It's actually quite hard to determine if a shell is interactive
between difference shells and environments.

Move the prompt to use direnv to the install_nix.sh script to condense
all possible prompt interactions into a single script.

https://linear.app/comm/issue/ENG-2847

Test Plan
rm -rf ~/Library/Caches/app.comm/install-direnv
brew uninstall direnv

./scripts/install_nix.sh

Diff Detail

Repository
rCOMM Comm
Branch
jonringer/move-prompts-install
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• jon created this revision.Feb 2 2023, 9:59 AM
Owners added a reviewer: Restricted Owners Package.Feb 2 2023, 9:59 AM
Herald added a subscriber: tomek. · View Herald TranscriptFeb 2 2023, 9:59 AM
Harbormaster completed remote builds in B16022: Diff 21869.Feb 2 2023, 10:16 AM
• jon requested review of this revision.Feb 2 2023, 10:16 AM
ashoat accepted this revision.Feb 2 2023, 11:42 AM

Mostly rubberstamping as I'm not very familiar with this code

varun accepted this revision.Feb 2 2023, 1:07 PM
This revision is now accepted and ready to land.Feb 2 2023, 1:07 PM
Closed by commit rCOMMaa565352bba0: [Nix] Only prompt for direnv usage on install (authored by • jon). · Explain WhyFeb 6 2023, 7:17 AM
This revision was automatically updated to reflect the committed changes.
• jon added a commit: rCOMMaa565352bba0: [Nix] Only prompt for direnv usage on install.

Revision Contents
Changeset List

PathSize
nix/
2 lines
scripts/
5 lines
2 lines

Diff 21869

View Options

nix/dev-shell.nix

  • This file was added.
// @flow
import * as React from 'react';
import { type SetState } from 'lib/types/hook-types';
import { type RelationshipButton } from 'lib/types/relationship-types';
import { type ThreadInfo } from 'lib/types/thread-types';
import type { UserInfo } from 'lib/types/user-types';
import ThreadSettingsRelationshipButton from './thread-settings-relationship-button.react';
import css from './thread-settings-relationship-tab.css';
type Props = {
+threadSettingsOperationInProgress: boolean,
+threadInfo: ThreadInfo,
tomekUnsubmitted
Not Done
Inline Actions

These props aren't used. Shouldn't this give a warning?

tomek: These props aren't used. Shouldn't this give a warning?
jacekAuthorUnsubmitted
Done
Inline Actions

Interesting. I thought flow should indicate warning for such variables.

jacek: Interesting. I thought flow should indicate warning for such variables.
ashoatUnsubmitted
Not Done
Inline Actions

There can be a good reason for having a prop that you never use: to force the component to rerender. For instance, we do something similar here

ashoat: There can be a good reason for having a prop that you never use: to force the component to…
+setErrorMessage: SetState<string>,
+relationshipButtons: $ReadOnlyArray<RelationshipButton>,
+otherUserInfo: UserInfo,
};
function ThreadSettingsRelationshipTab(props: Props): React.Node {
const { relationshipButtons, otherUserInfo, setErrorMessage } = props;
const buttons = React.useMemo(() => {
return relationshipButtons.map(action => {
return (
<ThreadSettingsRelationshipButton
key={action}
relationshipButton={action}
otherUserInfo={otherUserInfo}
setErrorMessage={setErrorMessage}
tomekUnsubmitted
Not Done
Inline Actions

In ThreadSettingsRelationshipButton setErrorMessage is string => void but here we use SetState<string>. Is flow ok with that? Is there a good reason for having different types?

tomek: In `ThreadSettingsRelationshipButton` `setErrorMessage` is `string => void` but here we use…
jacekAuthorUnsubmitted
Done
Inline Actions

Flow is ok here. I'll replace the type in ...Button with SetState<string>

jacek: Flow is ok here. I'll replace the type in `...Button` with `SetState<string>`
/>
);
});
}, [otherUserInfo, relationshipButtons, setErrorMessage]);
return <div className={css.container}>{buttons}</div>;
}
export default ThreadSettingsRelationshipTab;
View Options

scripts/install_nix.sh

  • This file was added.
// @flow
import * as React from 'react';
import { type SetState } from 'lib/types/hook-types';
import { type RelationshipButton } from 'lib/types/relationship-types';
import { type ThreadInfo } from 'lib/types/thread-types';
import type { UserInfo } from 'lib/types/user-types';
import ThreadSettingsRelationshipButton from './thread-settings-relationship-button.react';
import css from './thread-settings-relationship-tab.css';
type Props = {
+threadSettingsOperationInProgress: boolean,
+threadInfo: ThreadInfo,
tomekUnsubmitted
Not Done
Inline Actions

These props aren't used. Shouldn't this give a warning?

tomek: These props aren't used. Shouldn't this give a warning?
jacekAuthorUnsubmitted
Done
Inline Actions

Interesting. I thought flow should indicate warning for such variables.

jacek: Interesting. I thought flow should indicate warning for such variables.
ashoatUnsubmitted
Not Done
Inline Actions

There can be a good reason for having a prop that you never use: to force the component to rerender. For instance, we do something similar here

ashoat: There can be a good reason for having a prop that you never use: to force the component to…
+setErrorMessage: SetState<string>,
+relationshipButtons: $ReadOnlyArray<RelationshipButton>,
+otherUserInfo: UserInfo,
};
function ThreadSettingsRelationshipTab(props: Props): React.Node {
const { relationshipButtons, otherUserInfo, setErrorMessage } = props;
const buttons = React.useMemo(() => {
return relationshipButtons.map(action => {
return (
<ThreadSettingsRelationshipButton
key={action}
relationshipButton={action}
otherUserInfo={otherUserInfo}
setErrorMessage={setErrorMessage}
tomekUnsubmitted
Not Done
Inline Actions

In ThreadSettingsRelationshipButton setErrorMessage is string => void but here we use SetState<string>. Is flow ok with that? Is there a good reason for having different types?

tomek: In `ThreadSettingsRelationshipButton` `setErrorMessage` is `string => void` but here we use…
jacekAuthorUnsubmitted
Done
Inline Actions

Flow is ok here. I'll replace the type in ...Button with SetState<string>

jacek: Flow is ok here. I'll replace the type in `...Button` with `SetState<string>`
/>
);
});
}, [otherUserInfo, relationshipButtons, setErrorMessage]);
return <div className={css.container}>{buttons}</div>;
}
export default ThreadSettingsRelationshipTab;
View Options

scripts/prompt_direnv_macos.sh

  • This file was added.
// @flow
import * as React from 'react';
import { type SetState } from 'lib/types/hook-types';
import { type RelationshipButton } from 'lib/types/relationship-types';
import { type ThreadInfo } from 'lib/types/thread-types';
import type { UserInfo } from 'lib/types/user-types';
import ThreadSettingsRelationshipButton from './thread-settings-relationship-button.react';
import css from './thread-settings-relationship-tab.css';
type Props = {
+threadSettingsOperationInProgress: boolean,
+threadInfo: ThreadInfo,
tomekUnsubmitted
Not Done
Inline Actions

These props aren't used. Shouldn't this give a warning?

tomek: These props aren't used. Shouldn't this give a warning?
jacekAuthorUnsubmitted
Done
Inline Actions

Interesting. I thought flow should indicate warning for such variables.

jacek: Interesting. I thought flow should indicate warning for such variables.
ashoatUnsubmitted
Not Done
Inline Actions

There can be a good reason for having a prop that you never use: to force the component to rerender. For instance, we do something similar here

ashoat: There can be a good reason for having a prop that you never use: to force the component to…
+setErrorMessage: SetState<string>,
+relationshipButtons: $ReadOnlyArray<RelationshipButton>,
+otherUserInfo: UserInfo,
};
function ThreadSettingsRelationshipTab(props: Props): React.Node {
const { relationshipButtons, otherUserInfo, setErrorMessage } = props;
const buttons = React.useMemo(() => {
return relationshipButtons.map(action => {
return (
<ThreadSettingsRelationshipButton
key={action}
relationshipButton={action}
otherUserInfo={otherUserInfo}
setErrorMessage={setErrorMessage}
tomekUnsubmitted
Not Done
Inline Actions

In ThreadSettingsRelationshipButton setErrorMessage is string => void but here we use SetState<string>. Is flow ok with that? Is there a good reason for having different types?

tomek: In `ThreadSettingsRelationshipButton` `setErrorMessage` is `string => void` but here we use…
jacekAuthorUnsubmitted
Done
Inline Actions

Flow is ok here. I'll replace the type in ...Button with SetState<string>

jacek: Flow is ok here. I'll replace the type in `...Button` with `SetState<string>`
/>
);
});
}, [otherUserInfo, relationshipButtons, setErrorMessage]);
return <div className={css.container}>{buttons}</div>;
}
export default ThreadSettingsRelationshipTab;