Skip to content

Allows to set multiple base auth urls#61

Merged
itoys merged 7 commits into
masterfrom
itoys/multi-base-urls
Jan 10, 2024
Merged

Allows to set multiple base auth urls#61
itoys merged 7 commits into
masterfrom
itoys/multi-base-urls

Conversation

@itoys

@itoys itoys commented Jan 5, 2024

Copy link
Copy Markdown
Contributor

Allows to set multiple base auth urls

@itoys

itoys commented Jan 5, 2024

Copy link
Copy Markdown
Contributor Author

Ok, tests fail because a test app created with the rails, "~> 6", but the lib code works with any version, validated locally

@levenleven levenleven left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good

Ok, tests fail because a test app created with the rails, "~> 6", but the lib code works with any version, validated locally

Let's add ~> 6 requirement and bump gem's major. No reason to support older versions I think as they are EOL

Comment thread lib/chatops/controller.rb Outdated
@itoys

itoys commented Jan 8, 2024

Copy link
Copy Markdown
Contributor Author

Let's add ~> 6 requirement and bump gem's major. No reason to support older versions I think as they are EOL

This will stick rails version to 6.*, so the lib won't work with projects which use rails 7.*. We have an issue with spec/dummy app, which doesn't work with rails 7.

@itoys itoys marked this pull request as ready for review January 8, 2024 13:11

@levenleven levenleven left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@itoys itoys merged commit 807aeb0 into master Jan 10, 2024
@itoys itoys deleted the itoys/multi-base-urls branch January 10, 2024 11:58
@itoys itoys mentioned this pull request Jan 10, 2024
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