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.

Sign up for GitHub

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

Jump to bottom

Install deps.clj as workaround for cmd.exe #55

Merged
merged 4 commits into from
Jul 1, 2022
Merged

Install deps.clj as workaround for cmd.exe #55

merged 4 commits into from
Jul 1, 2022

Conversation

zharinov
Copy link
Contributor

@zharinov zharinov commented Jun 28, 2022 •
edited
Loading

zharinov
zharinov commented Jun 28, 2022
View reviewed changes
.github/workflows/smoke-tests.yml
@@ -3,11 +3,9 @@ name: Smoke tests
on: [push]

jobs:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for these. Can we stick to this yml formatting convention, which is forced by Prettier?

Copy link
Owner

Choose a reason for hiding this comment

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

No worries, I don't mind sticking with some convention if it improves readability. In addition - is it possible to configure prettier for the project? I think it is worth adding it as a command to run in package.json and include it into "run all" command as well. That way it will be clear for all contributors why yaml files are structured one way or another. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's definitely possible, and by the way Prettier will cover quite all the codebase here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But let's do this in separate PR, though

@zharinov
Copy link
Contributor Author

Tests are passing: https://github.com/zharinov/setup-clojure/runs/7086722617?check_suite_focus=true

DeLaGuardo
DeLaGuardo approved these changes Jul 1, 2022
View reviewed changes
Copy link
Owner

@DeLaGuardo DeLaGuardo left a comment

Choose a reason for hiding this comment

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

LGTM

@DeLaGuardo DeLaGuardo merged commit fc73f6f into DeLaGuardo:master Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DeLaGuardo DeLaGuardo DeLaGuardo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consider supporting deps.exe on Windows
2 participants
@zharinov @DeLaGuardo