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

Update README.md #89

Merged
merged 3 commits into from
Jul 4, 2023
Merged

Update README.md #89

merged 3 commits into from
Jul 4, 2023

Conversation

guitarrapc
Copy link
Contributor

Description

Improvement.

This PR explicitly guide user to use action/checkout with clean: false option.

Background

Please correct me if my understand is wrong. DeNA/setup-job-workspace-action is to achieve Jenkins checkout strategy "keeps .git for each node & job" on GitHub Actions by "keep .git for each runner & workspace". Therefore actions/checkout still expected to be used with clean: false option to avoid git clean -ffdx.

Current README.md not indicate actions/checkout to use with clean: false, however it may reduce confusion to explicitly show example.

Related issue:

Contributor License Agreements

@Kesin11
Copy link
Collaborator

Kesin11 commented Jun 30, 2023

@guitarrapc I agree with your diff.
However, I think that if you set clean: false, you will need to run git clean -ffd yourself. Without running git clean, files that generated by the previous run may affect the current run.
Therefore, we should also add run: git clean -ffd in example.

@srz-zumix @VeyronSakai What do you think? How do you do this in your actual Unity workflow?

@Kesin11
Copy link
Collaborator

Kesin11 commented Jun 30, 2023

@srz-zumix @VeyronSakai check-dist job blocked to merge. Since check-dist job will not trigger if the pull-request has only *.md diff, check-dist job should be removed from branch protection rules.

@Kesin11
Copy link
Collaborator

Kesin11 commented Jul 3, 2023

What do you think? How do you do this in your actual Unity workflow?

@srz-zumix @VeyronSakai Since I don't use Unity anymore, can you guys tell me about your actual workflow?

@Kesin11 Kesin11 added the documentation Improvements or additions to documentation label Jul 3, 2023
@srz-zumix
Copy link
Collaborator

We are using as a below

- name: Switch workspace
  uses: DeNA/setup-job-workspace-action@v2
  with:
    # Change path by build option
    suffix: "${{ (inputs.enable_XXX && '-XXX') || '' }}"
- uses: actions/checkout@v3
  with:
    # For clean everything
    clean: ${{ inputs.clean || false }}
- name: Git clean without Library
  run: git clean -xffd --exclude Library

@srz-zumix
Copy link
Collaborator

I thought it would be fine without clean:false in Usage.
Because this is an input for the checkout action and not for the setup-job-workspace-action.

How about adding a section like Example and adding the above Unity example there?

This reverts commit 0c1028d.
@guitarrapc guitarrapc requested a review from Kesin11 as a code owner July 4, 2023 03:14
@guitarrapc
Copy link
Contributor Author

@srz-zumix I've reverted usage, and added Example section as you mentioned. May I ask you review change?

Copy link
Collaborator

@Kesin11 Kesin11 left a comment

Choose a reason for hiding this comment

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

How about adding a section like Example and adding the above Unity example there?

I also agree it. LGTM 👍

Copy link
Collaborator

@srz-zumix srz-zumix left a comment

Choose a reason for hiding this comment

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

LGTM!

@srz-zumix srz-zumix merged commit 47b204a into DeNA:main Jul 4, 2023
8 checks passed
@guitarrapc guitarrapc deleted the patch-1 branch July 4, 2023 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants