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

Fixes #22104 - adapt to Rails 5.1 change tracking #5133

Merged
merged 1 commit into from
Feb 19, 2018
Merged

Fixes #22104 - adapt to Rails 5.1 change tracking #5133

merged 1 commit into from
Feb 19, 2018

Conversation

mmoll
Copy link
Contributor

@mmoll mmoll commented Jan 1, 2018

No description provided.

@theforeman-bot
Copy link
Member

Issues: #22104

@@ -57,13 +57,13 @@ def alias?
end

def fqdn_changed?
Copy link
Member

Choose a reason for hiding this comment

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

Since these two methods are meant to mimic active record methods, should we also change them to the new naming?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought its use were more spread over the code, but its indeed only in this file, so renaming was no problem 👍.

:hostname, :fqdn, :fqdn_changed?,
:fqdn_was, :shortname,
:hostname, :fqdn, :saved_change_to_fqdn?,
:fqdn_before_last_save, :shortname,
Copy link
Member

Choose a reason for hiding this comment

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

Do we actually use these anywhere? i think it may be leftovers from something, i couldn't find anywhere where these methods are called on a host

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed these.

@mmoll
Copy link
Contributor Author

mmoll commented Jan 3, 2018

test failures unrelated, AFAICT.

@mmoll
Copy link
Contributor Author

mmoll commented Jan 5, 2018

Setting to WOC, as GH-4987 exposed some more code paths with calls that need to get checked.

@mmoll
Copy link
Contributor Author

mmoll commented Feb 17, 2018

No movement in GH-4987, so I'm kindly asking to proceed here...
Rebased, all 💚.

Copy link
Member

@tbrisker tbrisker left a comment

Choose a reason for hiding this comment

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

Looks good. Even if this isn't every case it's better then nothing, let's merge now and fix any other issues we encounter. In any case this won't break before 5.2.

@tbrisker tbrisker merged commit 3de2d5a into theforeman:develop Feb 19, 2018
@mmoll mmoll deleted the 51_change_depr branch May 6, 2019 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants