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

Add working directory #14

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sondalex
Copy link

This PR adds working-directory for build and deployment process.

Copy link

darkest-pr bot commented Jan 19, 2025

👿 It is a travesty - a blundering mountain of hatred and rage.

@skywarth
Copy link
Owner

Hi @sondalex,

Thank you for such great contribution! I appreciate you for taking the time to prepare and submit it. I must admit that I'm not familiar with working-directory concept, so would you mind explaining it a bit for me to wrap my head around it. And please include some details about the feature itself: what does it do, who is it for, when would it come in handy etc.

If you have the time, it would be wonderful to add this working-directory parameter to the corresponding documentation section, so the dependents/users would be aware of it.

Lastly, it is important to preserve backward compatibility as with any software. So in order to ensure that we won't be disrupting the existing users with this change, we need to test it thoroughly.

I've already hooked one of my repos to your feature branch, and it worked fine with no issues:

I'll also test it with some of our current dependents, to see if it would break their workflow or not. My main focus will be the repos which pass various parameters to the action, to see if they conflict with each other or not. I'll update you on this afterwards, to let you know how it fares.

In the meantime, it would be great if you could address the necessities I've disclosed above.

I'm looking forward to working and merging this together with you, this will be a great addition.

Thank you very much!

P.S: Note to myself, don't forget to add it to the acknowledgement section

@sondalex
Copy link
Author

sondalex commented Jan 20, 2025

Hi @skywarth, thank you for your response and feedback. Your project is greatly helpful for rapidly deploying Vite applications to GitHub. I appreciate the effort you put into developing it!

The working-directory keyword allows you to specify the directory in which commands are executed within a GitHub Action workflow.

Currently, the main branch assumes that the repository's structure includes package.json at the root. By adding a working-directory option, users can manage different repository structures. For instance, consider the following structure:

app/package.json
app/...
README.md

With this structure, a corresponding GitHub Action configuration would look like this:

name: Vite GitHub Pages Deploy

on:
  push:
    branches: ["master", "main"]
  workflow_dispatch:

permissions:
  contents: read
  pages: write
  id-token: write

concurrency:
  group: "pages"
  cancel-in-progress: false

jobs:
  build:
    runs-on: ubuntu-latest
    environment:
      name: gh-pages
      url: ${{ steps.deploy_to_pages.outputs.github_pages_url }}
    steps:
      - name: Checkout
        uses: actions/checkout@v3
      - name: Deploy to GitHub Pages
        uses: sondalex/vite-github-pages-deployer@add_working_directory
        id: deploy_to_pages
        with:
          working-directory: ./app

In addition you I have updated the documentation accordingly.

@skywarth
Copy link
Owner

Looking good! Thank you for updating the documentation as well. I was wondering how does this differ from the existing build_path parameter? I saw that you maintained backward compatibility by using either build_path or working-directory, to assign as ARTIFACT_PATH.

Copy link

darkest-pr bot commented Jan 25, 2025

🕯️ A sister of battle. Pious and unrelenting.

@sondalex
Copy link
Author

The build_path parameter is expected to point to the Vite build output directory. The working-directory parameter sets the directory where commands are executed (equivalent to using the cd command in a shell).

Example:
If the working-directory is set to example/, the build process will run in the example/ directory, similar to executing cd example && yarn build.

Since yarn build is executed in the example/ directory, the artifact path must combine the working-directory (example/) and the default build_path (dist/). This means the final artifact path should be example/dist/.

Here’s an improved version of your note:

Note:
From an aesthetic point of view, this approach might feel somewhat cumbersome. An alternative solution is to set working-directory in the defaults section, as described in the GitHub Actions documentation

This method has the advantage of eliminating the need to define an intermediary environment variable for specifying the artifact directory.

@skywarth what are your thoughts on this ?

@skywarth
Copy link
Owner

Gotcha, it's crystal. Thank you for the explanation. It is a good use-case then.

About that alternative solution; I actually like the ring of it. It would simplify things a lot. But I'm considering if there is a catch where it could make it difficult for us, whether somehow there would a be case where we would need to be on another upper (or root) directory of the repository.

Let's stick to what we have for now, I wouldn't want you to invest even more effort into it.

I'll try to find a few dependents where they specified build_path parameter. From what I can see that is the only one that might be affected by this change. After finding these dependent repositories, I'll fork them and replace their respective action *.yml files to point to your fork (or this branch) and run the actions. Expected outcome is all should work as before.

I'm listing the test cases below for easier time tracking:

Test case Status Outcome & Remarks
Dependents with build_path parameter 🕒
Dependents with no parameter at all 🕒
Dependents with CNAME file in the repository 🕒
Repository specifically for this use case 🕒 @sondalex Can you help with this one please 🙏

I plan on doing the tests on weekend, I'll ping you once they are done. Afterwards if all goes well, we can wrap it up and merge it for good.

Two considerations you might wanna give a pondering about in the meantime:

  1. Maybe changing parameter name from working-directory might be a good idea, in case if there is a possibility for it to conflict with GitHub Action's standard working-directory parameter. And also to avoid confusion. How about working_path ? Just an idea, your call.
  2. This line feels a bit off for me. Are we adamant and certain that the build_path is always residing under the working_path directory? Is a scenario such as the one below possible? If so, this would be limiting 😞
.
├── src/ (target for `working_directory`)
├── build/ (target for `build_path`)
├── docs/
├── LICENSE
└── README.md

Thank you

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