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

Configure various dev tools #894

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

Steffo99
Copy link
Contributor

@Steffo99 Steffo99 commented Oct 21, 2024

This pull request pre-configures various dev tools to allow an easier onboarding for potential new developers.

In particular:

  • The .vscode and .idea directories are unexcluded, as they're not supposed to be excluded in first place

    • The .idea directory is left to be managed by IDE itself via the .idea/.gitignore file in it
  • The .vscode directory is pre-populated with settings to aid development in VSCode:

    • A run configuration to launch Phanpy on port 8080 and to open an instance of a Chromium-based browser in debug mode on it as soon as the server is done launching:

    • A recommendation for the github.vscode-github-actions extension that adds features to work on GitHub Actions files, such as proper syntax highlighting:

    • If the user has File Nesting enabled (it is not by default), its settings are extended to group together *.tsx, *.jsx, *.ts, *.js, *.module.css and *.css files with the same name:

    • The src/locales and node_modules directories are hidden by default from the file explorer and excluded from search, as they're not meant to be edited by hand

  • The .devcontainer directory is created with a basic but functional configuration to develop Phanpy in a development container, such as the ones provided by GitHub Codespaces

  • The .idea directory is pre-populated too with settings to aid development in Jetbrains' IDEs:

    • Project name has been set to "Phanpy" instead of whatever the current working directory is called

    • Code style settings are setup to be overridden by the project, which copies the Prettier features built-in in the IDE

      • Further code linting can be later performed before-commit by the external Prettier plugin
    • Basic module structure

    • A «Scope» is setup to mark localization files managed by Crowdin with a red colour, denoting that they should not be edited

  • The port for the development server has been changed from 5173 to 8080; while both are well-known for Vite development, only the latter is officially registered with the IANA for web development

  • Metadata is added to package.json to prevent accidental publishing ("private": true) and to have additional information if needed

  • The prettier-pr.yml GitHub Action is extended to allow it to automatically prettify files, instead of just checking them


Since this is a lot of opinionated stuff, if you're not comfortable with merging parts of it, I'd recommend you to cherry-pick the commits you think might be interesting, or to tell me which parts you like so that I can cherry-pick for you!

@Steffo99 Steffo99 marked this pull request as draft October 21, 2024 23:03
@Steffo99 Steffo99 closed this Oct 21, 2024
@Steffo99 Steffo99 reopened this Oct 21, 2024
@Steffo99 Steffo99 marked this pull request as ready for review October 21, 2024 23:06
@cheeaun
Copy link
Owner

cheeaun commented Nov 12, 2024

  1. Personally not a fan of editor-specific configs. There are too many editors out there (in the past, in the present, and will be many more in the future) with too many configs 🙈
  2. Prettier action is nice, tho' now I would prefer it to show warnings first (thus blocking the PR merge) instead of auto-fixing them — good for telling future contributors that they need this set up on their editors instead of relying on CI to auto-fix for them.
  3. Not sure if accidental publishing is common, but I don't think that should be our concern here(?) 🤔, re: package.json changes. I'm also not sure if all the metadata would be useful in other context as I prefer that file to be as minimalistic as possible.

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.

2 participants