Skip to content

Conversation

@aaccensi
Copy link
Contributor

@aaccensi aaccensi commented Jan 7, 2026

Description

As requested in quintel/etsource#3098, this PR is part of a set of 3 PRs with the goal of removing any trace of real estate module:

* Note that since Atlas is a Gem, we should follow this order:

  1. Merge the Atlas PR, get new commit SHA
  2. Update etengine/Gemfile and etsource/Gemfile with new Atlas ref, then retest and merge those PRs together (since they contain the removal of code that depends on the deleted Atlas functionality).

Type of change

  • Bug fix
  • New feature
  • Enhancement
  • Documentation
  • Maintenance

Checklist

  • I have tested these changes
  • I have updated documentation as needed
  • I have tagged the relevant people for review

Related Issues

References quintel/etsource#3098

@mabijkerk
Copy link
Member

If we update Atlas, we also have to update relevant Gemfiles.

@aaccensi
Copy link
Contributor Author

aaccensi commented Jan 7, 2026

If we update Atlas, we also have to update relevant Gemfiles.

Oh, you are right. this has some implications, we should follow this order:

  1. Merge the Atlas PR, get new commit SHA
  2. Update etengine/Gemfile and etsource/Gemfile with new Atlas ref, then retest and merge those PRs together (since they contain the removal of code that depends on the deleted Atlas functionality).

@mabijkerk
Copy link
Member

Yes, though we typically already test the Atlas commit of the branch before we even merge it.

@aaccensi
Copy link
Contributor Author

aaccensi commented Jan 7, 2026

Yes, though we typically already test the Atlas commit of the branch before we even merge it.

That is good to know. To get ahead of potential issues I did that locally for both ETSource and ETEngine.
I pointed the atlas gem to the new branch and I updated it. The specs for both repos still pass and seem to work fine.

@louispt1 louispt1 self-requested a review January 9, 2026 09:37
Copy link
Contributor

@louispt1 louispt1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@mabijkerk mabijkerk merged commit 6563179 into master Jan 9, 2026
1 check passed
@mabijkerk mabijkerk deleted the retire-real-estate branch January 9, 2026 10:23
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.

4 participants