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

Transition from Travis -> GH Actions and use Ameba #11

Merged
merged 7 commits into from
Jan 20, 2021

Conversation

stephendolan
Copy link
Collaborator

@stephendolan stephendolan commented Jan 19, 2021

This PR tackles the following:

  • Move from Travis CI to GitHub Actions. This is quickly becoming the most popular way to do CI in the GitHub ecosystem, and there are tons of built-in tools that will only get better with time. I've set this up identically to the Travis file, where we test against both the latest and nightly Crystal versions, allowing nightly to fail as an "experimental" version.
  • Add Dependabot capabilities to submit PRs for GitHub action library updates
  • Add badges to the README to show the current CI status and latest GitHub release version
  • Add Ameba linting to our specs. This helps to avoid bikeshedding on code style in favor of a popular community set of standards.
  • Removed crystal-coverage shard from development dependencies since it didn't appear to have been used previously with Travis, and doesn't currently compile.

@stephendolan stephendolan marked this pull request as draft January 19, 2021 14:25
@stephendolan
Copy link
Collaborator Author

stephendolan commented Jan 19, 2021

@confact, here's an output sample from Ameba to get a feel for what I'd be changing if we add it to the repo:

~/Repos/@oss/stripe.cr(sd-github-actions-switch) » ./bin/ameba                                                           stephen@stephens-mbp
Inspecting 94 files.

......................FFFFFFFFFFFFFFFFFFFFFFFFF.FFFFFFFFFFFFFFFFFFFF..........................

src/stripe/methods/core/customers/tax_ids/create_customer_tax_id.cr:19:7
[C] Style/RedundantReturn: Redundant `return` detected
> return Customer::TaxID.from_json(response.body)
  ^
src/stripe/methods/core/customers/tax_ids/retrieve_customer_tax_id.cr:9:7
[C] Style/RedundantReturn: Redundant `return` detected
> return Customer::TaxID.from_json(response.body)
  ^
src/stripe/methods/core/customers/update_customer.cr:18:5
[W] Lint/UselessAssign: Useless assignment to variable `default_source`
> default_source = default_source.as(Token).id if default_source.is_a?(Token)
  ^

Finished in 149.18 milliseconds 

94 inspected, 87 failures.

@confact
Copy link
Owner

confact commented Jan 19, 2021

@stephendolan, that's all fine. I know Ameba is starting to become standard in crystal, if not already. So go for it. Just happy that you want to spend time on this :)

@stephendolan
Copy link
Collaborator Author

That sounds great, @confact. One other question since I'm far from a semantic versioning expert...

Do you think this kind of refactor to satisfy Ameba is worth a version bump, or as long as all of the tests pass should we just leave the version alone?

@stephendolan
Copy link
Collaborator Author

Nevermind, I just realized that there's literally no point in cutting a release just for this CI and Ameba stuff :)

I did end up opening crystal-ameba/ameba#194 since I think there's an issue with Ameba that's preventing this from passing. Once that issue is resolved, I'll submit a follow up PR to remove the .ameba.yml.

@stephendolan stephendolan changed the title Transition from Travis -> GH Actions Transition from Travis -> GH Actions and use Ameba Jan 19, 2021
@stephendolan stephendolan marked this pull request as ready for review January 19, 2021 23:21
@stephendolan
Copy link
Collaborator Author

The nightly build will continue to fail until we figure out how to refactor Stripe::Error not to use JSON::Mapping and instead use JSON::Serializable. That said, I've marked nightly as experimental, so we should probably just merge this one and tackle that refactor in a separate PR.

This one just looks huge because I finally got crystal docs working, which allowed me to push an update to all of those committed doc files. Once we get automatic documentation working, they'll all be removed.

@confact
Copy link
Owner

confact commented Jan 20, 2021

@stephendolan this looks good to me, We can find some solution for the error later. It would be nice for everything to pass but let's also keep this small enough :)

Great job with docs as well, just prove it is good with some new eyes and energy to look things through!

Copy link
Owner

@confact confact left a comment

Choose a reason for hiding this comment

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

LGTM

@stephendolan stephendolan merged commit 8e45f6d into master Jan 20, 2021
@stephendolan stephendolan deleted the sd-github-actions-switch branch January 20, 2021 20:39
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.

2 participants