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

Support merging conflicts with moved files. #78

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

ashdnazg
Copy link
Member

@ashdnazg ashdnazg commented Aug 3, 2022

Previously, moved files were removed from one group and added to
another. This caused the build files referencing them to remain
dangling. This was solved by identifying files that were moved and using
the appropriate functions to move them.

@ashdnazg ashdnazg force-pushed the fix_dups branch 3 times, most recently from 3036188 to 692d0d5 Compare August 3, 2022 10:48
Copy link
Collaborator

@byohay byohay left a comment

Choose a reason for hiding this comment

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

F&S


def apply_file_changes(project, additions, removals)
addition_to_paths = additions.to_multi_h
removal_to_references = removals.to_multi_h.map do |change, paths|
Copy link
Collaborator

Choose a reason for hiding this comment

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

at this group it isn't clear that these are file references. Maybe we should filter them at this point with change["isa"] == "PBXFileReference"

Comment on lines +419 to +420
new_group = theirs_project.main_group.find_subpath("new_group", true)
file_reference = theirs_project.main_group.find_file_by_path("bar")
Copy link
Collaborator

Choose a reason for hiding this comment

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

The group can be created in base_project, then to find the file you can just do: theirs_project["bar"]

Copy link
Member Author

Choose a reason for hiding this comment

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

keeping as discussed


containing_group = path.empty? ? project.main_group : project[path]

if (removal_to_references[change] || []).compact.empty?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not doing compact when getting the references?
Otherwise you need to do compact in the line below: removal_to_references[change].compact.length

Copy link
Member Author

Choose a reason for hiding this comment

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

We actually should never compact, since we cannot move files which don't exist, as we no longer have their build file

@ashdnazg
Copy link
Member Author

ashdnazg commented Aug 3, 2022

F&Sed

Copy link
Collaborator

@byohay byohay left a comment

Choose a reason for hiding this comment

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

FSM

Copy link
Collaborator

@byohay byohay left a comment

Choose a reason for hiding this comment

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

FSM

end
end

def apply_group_removals(project, deletions)
Copy link
Collaborator

Choose a reason for hiding this comment

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

deletions -> removals

Previously, moved files were removed from one group and added to
another. This caused the build files referencing them to remain
dangling. This was solved by identifying files that were moved and using
the appropriate functions to move them.
@ashdnazg ashdnazg enabled auto-merge August 4, 2022 07:33
@ashdnazg ashdnazg merged commit 5ee0857 into Lightricks:main Aug 4, 2022
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.

None yet

2 participants