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

Fix: CI. #81

Merged
merged 2 commits into from Jan 19, 2023
Merged

Fix: CI. #81

merged 2 commits into from Jan 19, 2023

Conversation

dblock
Copy link
Contributor

@dblock dblock commented Jan 5, 2023

A bit of matrix work post dc5118f to make it all work, and re-added Danger.

Dropped support for Mongoid 3, couldn't make it work.

Closes #78

@dblock dblock force-pushed the fix-ci branch 20 times, most recently from 78f926f to 73c6b4b Compare January 5, 2023 15:30
@dblock dblock marked this pull request as ready for review January 5, 2023 15:38
@dblock
Copy link
Contributor Author

dblock commented Jan 5, 2023

@rmm5t all yours

@dblock dblock mentioned this pull request Jan 5, 2023
bundle _${{ matrix.entry.bundler }}_ install
- name: Bundle install with default bundler
if: ${{ !matrix.entry.bundler }}
run: bundle install
Copy link
Collaborator

Choose a reason for hiding this comment

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

[CONFUSED] ruby/setup-ruby supports a bundler option.

For example:

      - name: Set up Ruby
        uses: ruby/setup-ruby@v1
        with:
          ruby-version: ${{ matrix.ruby }}
          bundler: ${{ matrix.bundler }}

Why the extra work here to specify the bundler version? Also, for older ruby versions, just specify bundler 1.

Copy link
Contributor Author

@dblock dblock Jan 5, 2023

Choose a reason for hiding this comment

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

I cannot get the older rubies + rails to work with anything other than bundler 2.0.0.pre.3. So I chose the version recommended. That said, I think I can do bundler: 2.0.0.pre.3, I didn't realize that was a setting.'

Update: ruby-setup doesn't support 2.0.0.pre.3 format, but this works with the latest 1.x. ruby/setup-ruby#439

- { ruby: "2.7", mongodb: '4.4', gemfile: 'mongoid-5.0', bundler: '2.0.0.pre.3' }
- { ruby: "2.7", mongodb: '4.4', gemfile: 'mongoid-6.0' }
- { ruby: "2.7", mongodb: '4.4', gemfile: 'mongoid-7.0' }
- { ruby: "3.1", mongodb: '4.4', gemfile: 'mongoid-8.0' }
Copy link
Collaborator

Choose a reason for hiding this comment

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

[CONFUSED] I don't understand this refactor of the matrix. I'd rather see a traditional build matrix and possible include or exclude keys for edge cases.

i.e. This entry array doesn't make sense to me. What am I misunderstanding?

Copy link
Contributor Author

@dblock dblock Jan 5, 2023

Choose a reason for hiding this comment

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

I find this a lot cleaner and more declarative because we benefit little from a matrix.

  1. This is a 6x4 matrix and we should also be testing multiple MongoDB versions, so this will be 6x4x3 = 72 builds where about 75% are completely redundant. Even here, I started with a regular matrix and ended up with a long list of excludes, this avoids doing that.
  2. For these older Rubies we need a different version of bundler (2.0.0.pre.3). This is yet another parameter then, matrix explodes.

Copy link
Contributor Author

@dblock dblock left a comment

Choose a reason for hiding this comment

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

My goal is get this back to green since dc5118f was merged without CI. Happy to accommodate whatever structure you prefer, lmk.

- { ruby: "2.7", mongodb: '4.4', gemfile: 'mongoid-5.0', bundler: '2.0.0.pre.3' }
- { ruby: "2.7", mongodb: '4.4', gemfile: 'mongoid-6.0' }
- { ruby: "2.7", mongodb: '4.4', gemfile: 'mongoid-7.0' }
- { ruby: "3.1", mongodb: '4.4', gemfile: 'mongoid-8.0' }
Copy link
Contributor Author

@dblock dblock Jan 5, 2023

Choose a reason for hiding this comment

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

I find this a lot cleaner and more declarative because we benefit little from a matrix.

  1. This is a 6x4 matrix and we should also be testing multiple MongoDB versions, so this will be 6x4x3 = 72 builds where about 75% are completely redundant. Even here, I started with a regular matrix and ended up with a long list of excludes, this avoids doing that.
  2. For these older Rubies we need a different version of bundler (2.0.0.pre.3). This is yet another parameter then, matrix explodes.

bundle _${{ matrix.entry.bundler }}_ install
- name: Bundle install with default bundler
if: ${{ !matrix.entry.bundler }}
run: bundle install
Copy link
Contributor Author

@dblock dblock Jan 5, 2023

Choose a reason for hiding this comment

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

I cannot get the older rubies + rails to work with anything other than bundler 2.0.0.pre.3. So I chose the version recommended. That said, I think I can do bundler: 2.0.0.pre.3, I didn't realize that was a setting.'

Update: ruby-setup doesn't support 2.0.0.pre.3 format, but this works with the latest 1.x. ruby/setup-ruby#439

@dblock dblock force-pushed the fix-ci branch 3 times, most recently from 5d226f9 to 3786f4d Compare January 5, 2023 22:58
@rmm5t
Copy link
Collaborator

rmm5t commented Jan 19, 2023

Thanks for this. Sorry for the delay on the merge.

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.

Add mongoid 8 support
2 participants