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

Allow Ransack to be tested with Rails main branch #1192

Merged
merged 4 commits into from
Jan 23, 2021
Merged

Allow Ransack to be tested with Rails main branch #1192

merged 4 commits into from
Jan 23, 2021

Conversation

yahonda
Copy link
Contributor

@yahonda yahonda commented Dec 30, 2020

This pull request allows Ransack to be tested with Rails master branch aka 6.2.0.alpha.

Let me explain some background for this change.

Ransack (polyamorous) depends on some Rails internal APIs and Rails internal APIs change without deprecation warnings. In the long term, Ransack (polyamorous) may not depend on these internal API, but as of now, what we can do is run CI against Rails master branch so that we will able to know which commits 'break' Ransack behavior.

@yahonda
Copy link
Contributor Author

yahonda commented Dec 30, 2020

Let me take a look at [!] There was an error parsing Gemfile: Malformed version number string master. Bundler cannot continue. error.

@yahonda
Copy link
Contributor Author

yahonda commented Dec 30, 2020

[!] There was an error parsing Gemfile: Malformed version number string master. Bundler cannot continue. error. has been addressed via 0d60b9a

Copy link
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

I'm cool with this and I agree with you that it'd be nice to discover breakage as soon as it happens 👍.

What I dislike is the fact of checking this for every PR, because I like PRs to be green for merging them, and doing this makes a PR potentially red at any time, without it being caused by the changes in a PR. My personal practice is that a PR should be green if and only if the changes in the PR are good.

What I like to do is using a daily scheduled worklfow for this. Currently github doesn't support builtin notifications for this, so if you want notifications you need to do a bit more work, but just having it run daily and being able to check the status in the actions tab helps I believe.

@yahonda
Copy link
Contributor Author

yahonda commented Dec 30, 2020

Thanks for the review and I understand what you do not like. Let me create another workflow called "cronjob" or something like that which executes daily cron job against Rails master branch.

@yahonda yahonda changed the title Allow Ransack to be tested with Rails master branch Allow Ransack to be tested with Rails main branch Jan 17, 2021
@scarroll32
Copy link
Member

Are you happy for this to be merged now @deivid-rodriguez ?

Copy link
Contributor

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Yes!

@scarroll32 scarroll32 merged commit 7fc3166 into activerecord-hackery:master Jan 23, 2021
deivid-rodriguez pushed a commit that referenced this pull request Apr 12, 2022
)

Refer to #1192 and #1205 for the similar change to support 6.2.0.alpha
and 7.0.0.alpha

This commit should make cronjob CI green.
@yahonda yahonda mentioned this pull request Apr 12, 2022
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.

3 participants