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 reset and clean for submodules #628

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

Conversation

XavierChapron
Copy link

Related to #358

@XavierChapron
Copy link
Author

@ericsciple Please have a look.

@jeppefrandsen
Copy link

Any news on this issue? Would be nice to get fixed 🍻

@AceCoderLaura
Copy link

I might pick up this PR and finish it. This is causing disruptions for my company.

@AceCoderLaura
Copy link

This PR appears to break two tests, I will attempt to remedy this.

@AceCoderLaura
Copy link

I think I've addressed the review items as well as passed the tests. Please fast forward this branch to https://github.com/AceCoderLaura/checkout/tree/finishing-628 if my changes are satisfactory.

@XavierChapron
Copy link
Author

I think I've addressed the review items as well as passed the tests. Please fast forward this branch to https://github.com/AceCoderLaura/checkout/tree/finishing-628 if my changes are satisfactory.

Hello, thank you!
You have many whitespace change in your commit AceCoderLaura@8328374
Can you change this? Or if the change are legetimate, maybe create a specific commit?

Then I will be pleased to cherry-pick your commits. And I hope someone will review and merge this anytime soon!

@AceCoderLaura
Copy link

Ah yes, my git configuration wasn't setup to convert line endings back to UNIX-style. I'll fix it some time today if I get some time.

@AceCoderLaura
Copy link

I've pushed a version with only the changes to the TypeScript source files here: https://github.com/AceCoderLaura/checkout/tree/finishing-628-no-compile I will have to ask you to build the project yourself because I've run out of time to fix the line endings issue. Hope this helps!

I had to add `"newLine": "LF"` to the tsconfig.json. I also had to discard a change to `module.exports` on line 4232 because I'm pretty sure it had nothing to do with my changes.
@AceCoderLaura
Copy link

I got some more time to work on this today and successfully transpiled the change to index.js without any disruptions to the line endings.

@XavierChapron
Copy link
Author

I got some more time to work on this today and successfully transpiled the change to index.js without any disruptions to the line endings.

Thanks a lot, I've included your commit in this PR.

@pefribeiro
Copy link

Hitting the same issue. Will this ever be merged?

@AceCoderLaura
Copy link

We're still using this fork but it's getting stale and it now has conflicts with the main branch. I haven't tried the latest version, does anyone know if this is still an issue in the main branch? If it is, I'll make some time to update the forked version.

@OscarVanL
Copy link

I ran into this issue recently. For me, a file in a submodule got modified by a previous run and then was present in the next run, causing issues.

It would be nice if this got merged so I can have reliable CI without adding hacks.

This was my workaround by the way:

      - uses: actions/checkout@v3
        with:
          submodules: true
      - name: Submodule cleanup fix  # Bodge for https://github.com/actions/checkout/issues/358
        run: |
          git submodule foreach --recursive git clean -ffdx
          git submodule foreach --recursive git reset --hard

@Hish15
Copy link

Hish15 commented Jan 21, 2025

What is preventing this from being merged ?

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.

7 participants