Skip to content

Conversation

@ColinKennedy
Copy link
Contributor

Closes: #57

Example:

      - uses: JohnnyMorganz/stylua-action@vFutureReleaseAfterThisPrIsMerged
        with:
          version: latest
          token: ${{ secrets.GITHUB_TOKEN }}
          args: false

@JohnnyMorganz
Copy link
Owner

Thanks, this makes sense to me!

Do you mind adding a quick smoketest to the github CI check with args: false example? (And then a later step which uses the cached artifact)

Also, could you update the README?

@ColinKennedy ColinKennedy force-pushed the remove_exec_test branch 2 times, most recently from ec78d9f to 1204822 Compare January 13, 2025 01:25
@ColinKennedy
Copy link
Contributor Author

I added a smoketest that adds a file that contains a stylua issue then used cmp to verify that the file is unchanged. Will that work as a test? I've also updated the README.md

@JohnnyMorganz
Copy link
Owner

Yup, looks good.

Just to double clarify, how do you use stylua manually afterwards - I assume it's available on the PATH so you can just reference stylua in a run step?

@ColinKennedy
Copy link
Contributor Author

I assume it's available on the PATH so you can just reference stylua in a run step?

Yes, I call stylua indirectly, via make. See https://github.com/ColinKennedy/nvim-best-practices-plugin-template/blob/main/Makefile. Repository structures tend to vary but using make as a bit of indirection means I can call make check-stylua or make stylua on any repo and it will always do the right thing, which I find that consistency useful. That's why

@ColinKennedy
Copy link
Contributor Author

@JohnnyMorganz I'm not sure what caused https://github.com/JohnnyMorganz/stylua-action/actions/runs/12738986339/job/35542812438?pr=58 to fail but in case it was because of a difference in newline, I removed that code. Would you let me know if you need me to do anything else for this PR?

@JohnnyMorganz
Copy link
Owner

Looks like it might be line endings issues. I can take a look at that on the weekend, but otherwise LGTM

@ColinKennedy
Copy link
Contributor Author

@JohnnyMorganz any chance we can move forward with this PR?

@JohnnyMorganz
Copy link
Owner

Hey, very sorry for the delay here. Completely forgot about this.

I've made some very small tweaks to the README and test, but it should be good now.

Turns out the failure was because we need to commit the dist files (completely forgot about this...)

Thank you for the contribution!

@JohnnyMorganz JohnnyMorganz merged commit 479972f into JohnnyMorganz:main Apr 1, 2025
5 checks passed
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.

Feature Request: Allow install+cache-only mode

2 participants