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

add justfile #1513

Merged
merged 10 commits into from
Jan 16, 2025
+59 −16
Merged

add justfile #1513

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
19025d7
add justfile
xavdid-stripe Jan 9, 2025
a43a7ee
add just to ci and split out commands
xavdid-stripe Jan 9, 2025
ff4ddbb
Fix justfile
xavdid-stripe Jan 9, 2025
fc00553
update readme and CI
xavdid-stripe Jan 10, 2025
ddc3fbc
update justfile
xavdid-stripe Jan 10, 2025
ae62982
update CI
xavdid-stripe Jan 10, 2025
fd2ac60
Merge branch 'master' into DEVSDK-2325
xavdid-stripe Jan 13, 2025
24769db
update justfile import
xavdid-stripe Jan 16, 2025
f53dc83
Merge branch 'master' into DEVSDK-2325
xavdid-stripe Jan 16, 2025
d165ad1
remove unused block
xavdid-stripe Jan 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,15 @@ jobs:
matrix:
ruby-version: [2.3, 2.4, 2.5, 2.6, 2.7, '3.0', 3.1, 3.2, '3.3', jruby-9.4.0.0, truffleruby-head]
steps:
- uses: extractions/setup-just@v2
- uses: actions/checkout@v3
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
ruby-version: ${{ matrix.ruby-version }}
- uses: stripe/openapi/actions/stripe-mock@master
- name: test
run: make ci-test
run: just test typecheck
xavdid-stripe marked this conversation as resolved.
Show resolved Hide resolved
env:
GITHUB_TOKEN: ${{ secrets.github_token }}

Expand Down
2 changes: 2 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# NOTE: this file is deprecated and slated for deletion; prefer using the equivalent `just` commands.

.PHONY: update-version codegen-format test ci-test
update-version:
@echo "$(VERSION)" > VERSION
Expand Down
29 changes: 29 additions & 0 deletions justfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
set quiet

import? '../sdk-codegen/justfile'

_default:
just --list --unsorted

install *args:
bundle install {{ if is_dependency() == "true" {"--quiet"} else {""} }} {{ args }}

# mass-format files
format: install
bundle exec rubocop -o /dev/null --autocorrect

test: install
bundle exec rake test

typecheck:
{{ if semver_matches(`ruby -e "puts RUBY_VERSION"`, ">=2.7") == "true" { \
Copy link
Member Author

Choose a reason for hiding this comment

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

just supports semver checks out of the box, so I got to simplify this block!

Copy link
Contributor

Choose a reason for hiding this comment

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

just to confirm, is this the "runs directly" referenced from the Rakefile? I think the same concerns I had with node/package.json applies; does having it in the Rakefile support some base level of development that someone who isnt comfortable with just can still benefit from?

Copy link
Member Author

Choose a reason for hiding this comment

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

just to confirm, is this the "runs directly" referenced from the Rakefile?

no, that's that we call bundle exec rubocop in just in CI instead of bundle exec rake rubocop in CI.

does having [rubocop] in the Rakefile ...

probably a little, but it's also a very common ruby pattern to bundle exec <tool> to run local tools.

Having it in the rakefile means a user could bundle exec rake rubocop instead of bundle exec rubocop. That would be useful if we were adding other arguments or configuration in that task, but since it's literally just calling rubocop like the user would, involving rake with rubocop at all strikes me as unnecessary abstraction.

Copy link
Member Author

Choose a reason for hiding this comment

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

This comment is referencing how much clear this is compared to the same check in the Makefile, which is a bunch of extra bash code (compared to just, which supports semver checks directly)

Copy link
Contributor

Choose a reason for hiding this comment

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

that makes sense to me

"bundle exec srb tc" \
Copy link
Member Author

Choose a reason for hiding this comment

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

this whole line gets executed if the expression is truthy

Copy link
Contributor

Choose a reason for hiding this comment

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

can you clarify this? im not sure what the comment is calling out

Copy link
Member Author

@xavdid-stripe xavdid-stripe Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Mostly just explaining the just syntax. If the semver_matches call above is "true", then just runs bundle exec src tc. I found it a little counter intuitive, because it looks like the expression is returning a string (which it is, but that string just becomes a line in the file). Was just adding context to the reviewer

Copy link
Contributor

Choose a reason for hiding this comment

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

ohhh. so this conditionally inserts "bundle exec srb tc" into the typecheck: step when it is run/evaluated, and thats what just executes. is that the standard way to do conditional execution in just, or is that something special we had to do here?

Copy link
Member Author

Choose a reason for hiding this comment

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

ohhh. so this conditionally inserts "bundle exec srb tc" into the typecheck: ste

exactly!

is that the standard way to do conditional execution in just

Yes, or something like:

should_sorbet := semver_matches(`ruby -e "puts RUBY_VERSION"`, ">=2.7")
typecheck:
    [ {{ should_sorbet }} == "true" ] && bundle exec srb tc

But then we shell out to ruby every time someone runs just, which has a small performance implication. Otherwise, they're equivalent!

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense to me, and it seems readable on its face.

} else { \
"echo \"Ruby version < 2.7, skipping srb tc\"" \
} }}

# called by tooling
[private]
update-version version:
echo "{{ version }}" > VERSION
perl -pi -e 's|VERSION = "[.\-\w\d]+"|VERSION = "{{ version }}"|' lib/stripe/version.rb
Loading