Skip to content

refactor: move vscode settings setup to pnpm postinstall hook#7834

Open
wesleybl wants to merge 3 commits intomainfrom
refactor/vscode-settings
Open

refactor: move vscode settings setup to pnpm postinstall hook#7834
wesleybl wants to merge 3 commits intomainfrom
refactor/vscode-settings

Conversation

@wesleybl
Copy link
Member

  • Remove vscodesettings.js execution from make install targets
  • Add .vscode/settings.json to .gitignore
  • Remove .vscode/settings.json from git tracking
  • Add postinstall hook to package.json to run vscodesettings.js
  • Fix script to properly check if settings.json exists before reading

Therefore, when we run make install, settings.json will not appear as modified in git, and the settings will be added to the file. They will also be added when only pnpm install is run.

- Remove vscodesettings.js execution from make install targets
- Add .vscode/settings.json to .gitignore
- Remove .vscode/settings.json from git tracking
- Add postinstall hook to package.json to run vscodesettings.js
- Fix script to properly check if settings.json exists before reading
@wesleybl
Copy link
Member Author

See: #7758 (comment)

Copy link
Member

@davisagli davisagli left a comment

Choose a reason for hiding this comment

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

Seems ok to me

@sneridagh
Copy link
Member

@wesleybl I still haven't taken action on this one, because I'm still not sure about commiting the .vscode/settings.json. I'd like to bring this up in the next meeting.

My take: leave it as it was.

If we decide otherwise, then the postinstall hook is unnecessary and running that script is useless, and should be removed.

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.

3 participants