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

Add Active Support 7 support #314

Merged
merged 3 commits into from
Dec 21, 2021
Merged

Add Active Support 7 support #314

merged 3 commits into from
Dec 21, 2021

Conversation

tagliala
Copy link
Contributor

Allow Active Support 7.0

Also:

  • Test against latest ruby versions
  • Test against Rails 6.1
  • Use setup-ruby@v1 action
  • Update Gemfile.lock

@tagliala tagliala mentioned this pull request Dec 20, 2021
Closed
@javierjulio javierjulio mentioned this pull request Dec 21, 2021
Closed
javierjulio
javierjulio reviewed Dec 21, 2021
View reviewed changes
Copy link
Member

@javierjulio javierjulio left a comment

Choose a reason for hiding this comment

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

Just two small changes and then I'll approve and merge. Thanks!

.gitignore Outdated Show resolved Hide resolved
spec/rails/integration/forms_spec.rb Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@tagliala tagliala marked this pull request as draft December 21, 2021 08:46
@tagliala
Allow Active Support 7.0
aedb879 
Also:
- Test against latest ruby versions
- Test against Rails 6.1
- Use setup-ruby@v1 action
- Update Gemfile.lock
@tagliala
Use selector to detect authenticity token input
b2a867d 
Rails 7.0 adds the autocomplete attribute to hidden fields. By
switching to a selector, we can test both Rails 6 and 7
@tagliala tagliala mentioned this pull request Dec 21, 2021
Merged
@tagliala tagliala marked this pull request as ready for review December 21, 2021 11:38
javierjulio
javierjulio approved these changes Dec 21, 2021
View reviewed changes
Copy link
Member

@javierjulio javierjulio left a comment

Choose a reason for hiding this comment

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

Thanks! Would you mind also updating the CHANGELOG.md file like you did with InheritedResources? I'll merge right after as this is ready.

@tagliala
Copy link
Contributor Author

Hi, I've read this comment in the changelog and I thought it was kinda of automatic

<!--- The following link definition list is generated by PimpMyChangelog --->

I've manually did the change

@javierjulio
Copy link
Member

Oh it isn't, that (PimpMyChangelog) doesn't appear to be used anymore so it's safe to ignore. Thanks again!

@javierjulio javierjulio merged commit e4970e6 into activeadmin:master Dec 21, 2021
@tagliala tagliala mentioned this pull request Dec 21, 2021
Merged
11 tasks
@tagliala
Copy link
Contributor Author

I'm asking if it is possible to release a new version of this gem, for the following reasons:

@bbugh
Copy link

bbugh commented Jan 12, 2022

@javierjulio anything we can do to help get a new release of this? ActiveAdmin is the only thing blocking us from our Rails 7 upgrade and this is an important step forward (thanks to the great work of @tagliala too)

@tagliala
Copy link
Contributor Author

ActiveAdmin is the only thing blocking us from our Rails 7 upgrade and this is an important step forward (thanks to the great work of @tagliala too)

Thanks!

OT: there are new issues with Active Admin and Rails 7 not covered by specs, with information on how to reproduce available at activeadmin/activeadmin#7235 and I personally have no idea on how to fix them

Any help would be appreciated

@technicalpickles
Copy link

Are there still arbre related issues on activeadmin/activeadmin#7235 ? I was looking and it seems to only be the issue with getting consts.

@tagliala
Copy link
Contributor Author

tagliala commented Feb 4, 2022

Are there still arbre related issues on activeadmin/activeadmin#7235 ?

A new version of arbre hasn't been released yet, so I can't mark at completed the checkbox at activeadmin/activeadmin#7235

@tomascco tomascco mentioned this pull request Oct 5, 2023
Merged
@tagliala tagliala deleted the feature/activesupport-7 branch November 29, 2023 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@javierjulio javierjulio javierjulio approved these changes

@edek37 edek37 edek37 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.

None yet
5 participants
@tagliala @javierjulio @bbugh @technicalpickles @edek37