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 Ruby "method redefined" warning #433

Merged

Conversation

mattbrictson
Copy link

When Ruby warnings are enabled, this is printed when loading the sitemap_generator gem:

sitemap_generator-6.3.0/lib/sitemap_generator/templates.rb:16):1: warning: method redefined; discarding old sitemap_sample

This is because an attr reader (e.g. sitemap_sample) was being defined via attr_accessor and then immediately redefined with define_method.

Fix by using attr_writer instead of attr_accessor, so that the attr reader is not defined twice.

@mattbrictson
Copy link
Author

The failing integration tests can be fixed by merging #434.

@mattbrictson mattbrictson changed the title Fix Ruby "method refined" warning Fix Ruby "method redefined" warning May 24, 2024
@n-rodriguez
Copy link
Collaborator

@mattbrictson Hi there! can you please rebase your branch? Thank you!

When Ruby warnings are enabled, this is printed when loading the
sitemap_generator gem:

> sitemap_generator-6.3.0/lib/sitemap_generator/templates.rb:16):1: warning: method redefined; discarding old sitemap_sample

This is because an attr reader (e.g. `sitemap_sample`) was being defined
via `attr_accessor` and then immediately redefined with `define_method`.

Fix by using `attr_writer` instead of `attr_accessor`, so that the attr
reader is not defined twice.
@mattbrictson mattbrictson force-pushed the fix-method-redefined-warning branch from 36a0e6d to b7f8fb8 Compare November 5, 2024 01:13
@mattbrictson
Copy link
Author

@n-rodriguez rebased ✔️

@n-rodriguez n-rodriguez merged commit cfcffd1 into kjvarga:master Nov 5, 2024
48 checks passed
@n-rodriguez
Copy link
Collaborator

@mattbrictson Thank you!

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