Skip to content

Support Ruby 3 #949

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

Merged
merged 4 commits into from
Apr 6, 2021
Merged

Conversation

magneland
Copy link
Contributor

No description provided.

@magneland magneland force-pushed the ruby3 branch 3 times, most recently from fc71a46 to 3aba47a Compare March 26, 2021 23:57
Bump CircleCI ruby orb to `circleci/ruby@1.1.2`
Combine `lint` and `test` / `test_jruby` jobs.
JRuby job now installs "java" gems correctly.

Heavily inspired by `rubocop-rails` config.

See: https://github.com/rubocop/rubocop-rails/blob/f3a00977d8621a2af037cc8625cd2734e2c11a56/.circleci/config.yml
@magneland magneland changed the title Bump CircleCI orb to circleci/ruby@1.0 Support Ruby 3 Mar 27, 2021
@magneland
Copy link
Contributor Author

Ruby 3 contains a number of changes:
https://www.ruby-lang.org/en/news/2020/12/25/ruby-3-0-0-released/

However, the only relevant change appears to be:
https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/

This PR changes the CircleCI config to support Ruby 3, but also refactors the config.
It's unclear to me why the old checks are still Required - maybe due to some GitHub project settings?

@magneland magneland marked this pull request as ready for review March 27, 2021 01:23
Copy link
Contributor Author

@magneland magneland left a comment

Choose a reason for hiding this comment

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

Fixes #928

@@ -156,7 +156,7 @@ def include_issues_by_labels(issues)
filter_wo_labels(filtered_issues)
end

# @param [Array] issues Issues & PRs to filter when without labels
# @param [Array] items Issues & PRs to filter when without labels
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's debatable whether this commit belongs in this PR.
I would probably prefer a Rubocop cop or something that auto-corrects this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yard-junk is a tool for this. Finding errors and presenting them in a small report.

@@ -382,6 +388,7 @@ def commits_in_tag(sha, shas = Set.new)
shas
end

# @param [Object] indata
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are super cool additions, and type descriptions in Yard can use a comma to separate each matching type like [Array, Hash] etc.

@@ -442,6 +452,7 @@ def iterate_pages(client, method, *arguments, parent: nil, **options)
# This is wrapper with rescue block
#
# @return [Object] returns exactly the same, what you put in the block, but wrap it with begin-rescue block
# @param [Proc] block
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yard probably has some notation for you here, specific to blocks. We don't want the docs to show that this method takes a positional argument. Does the rendered Yard docs HTML show that?

@@ -483,6 +496,7 @@ def retry_callback
end
end

# @param [Object] msg
Copy link
Collaborator

Choose a reason for hiding this comment

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

This type is String.

@@ -453,6 +464,8 @@ def check_github_response(&block)
end

# Presents the exception, and the aborts with the message.
# @param [Object] message
# @param [Object] error
Copy link
Collaborator

Choose a reason for hiding this comment

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

Error is Exception I guess.

@@ -453,6 +464,8 @@ def check_github_response(&block)
end

# Presents the exception, and the aborts with the message.
# @param [Object] message
Copy link
Collaborator

Choose a reason for hiding this comment

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

Message is a String.

#
# @yield [Sawyer::Resource] An OctoKit-provided response (which can be empty)
#
# @return [void]
# @param [Hash] options
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: this param should be placed before th return Yard directive.

@@ -357,6 +361,8 @@ def fetch_tag_shas(tags)

private

# @param [Set] shas
# @param [Object] sha
Copy link
Collaborator

Choose a reason for hiding this comment

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

The sha is String, I think.

@@ -335,6 +338,7 @@ def default_branch
@default_branch ||= client.repository(user_project)[:default_branch]
end

# @param [Object] name
Copy link
Collaborator

Choose a reason for hiding this comment

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

Name is String.

@@ -170,6 +170,7 @@ def filter_wo_labels(items)
end

# @todo Document this
# @param [Object] issues
Copy link
Collaborator

Choose a reason for hiding this comment

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

This enumerable is probably an Array

@olleolleolle
Copy link
Collaborator

Hi, I made copious inline comments.

Also: yay! A great move to begin using the Ruby Orb better.

@olleolleolle
Copy link
Collaborator

In order to use this, I will move forward without fixing the pointed-out YARD annotations.

@olleolleolle olleolleolle merged commit b7bf6c1 into github-changelog-generator:master Apr 6, 2021
@magneland
Copy link
Contributor Author

In order to use this, I will move forward without fixing the pointed-out YARD annotations.

Cool thanks, makes sense. I saw your comments about YARD but did not have time to go through them all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants