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 regression in ModuleMapMapper due to outdated Graph properties #6221

Merged
merged 1 commit into from Apr 22, 2024

Conversation

fortmarek
Copy link
Member

Short description πŸ“

The latest release contains a regression due to: #6218

The regression happened because:

  • ModuleMapMapper is no longer called that early in the process before other mappers update the graph
  • We made some bad design decisions in the past and targets and dependencies are available in two ways. In mappers, we don't always update both properties (in this case Graph.dependencies and Target.dependencies) which makes the other one outdated. That's exactly what happened here.

The solution for now is to access target dependencies through GraphTraverser but long-term, we should get rid of these duplicated properties altogether, so bugs like this are no longer possible.

How to test the changes locally 🧐

Run tuist generate when using binaries.

Contributor checklist βœ…

  • The code has been linted using run mise run lint:fix
  • The change is tested via unit testing or acceptance testing, or both
  • The title of the PR is formulated in a way that is usable as a changelog entry
  • In case the PR introduces changes that affect users, the documentation has been updated

Reviewer checklist βœ…

  • The code architecture and patterns are consistent with the rest of the codebase
  • Reviewer has checked that, if needed, the PR includes the label changelog:added, changelog:fixed, or changelog:changed, and the title is usable as a changelog entry

@fortmarek fortmarek added the changelog:fixed PR will be listed in the Fixed section of CHANGELOG label Apr 22, 2024
@fortmarek fortmarek requested a review from pepicrft April 22, 2024 17:30
@pepicrft pepicrft merged commit 50460e8 into main Apr 22, 2024
1 of 8 checks passed
@pepicrft pepicrft deleted the fix/module-mapper-regression branch April 22, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:fixed PR will be listed in the Fixed section of CHANGELOG
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants