Skip to content
This repository has been archived by the owner on Nov 3, 2022. It is now read-only.

436 processing changes #482

Merged
merged 17 commits into from
Mar 28, 2021
Merged

436 processing changes #482

merged 17 commits into from
Mar 28, 2021

Conversation

bonndan
Copy link
Collaborator

@bonndan bonndan commented Mar 16, 2021

This PR allows tracking of landscape component (Group, Item, Relation) changes on field level in order to highlight changes in the UI.

requirement for #436

@bonndan bonndan requested a review from mfbieber March 16, 2021 20:35
@mfbieber mfbieber mentioned this pull request Mar 27, 2021
Copy link
Contributor

@mfbieber mfbieber left a comment

Choose a reason for hiding this comment

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

Looks fine overall 👍

src/main/java/de/bonndan/nivio/input/Indexer.java Outdated Show resolved Hide resolved
src/main/java/de/bonndan/nivio/model/ComponentDiff.java Outdated Show resolved Hide resolved
src/main/java/de/bonndan/nivio/model/ComponentDiff.java Outdated Show resolved Hide resolved
*/
public static void compareStrings(String a, String b, String key, List<String> changes) {
if (!org.apache.commons.lang3.StringUtils.equals(a, b)) {
changes.add(key + " changed to: " + b);
Copy link
Contributor

Choose a reason for hiding this comment

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

Any thoughts on thread safety and reads and writes to the changes List in nivio? Does it matter and could we run into problems?

Copy link
Collaborator Author

@bonndan bonndan Mar 28, 2021

Choose a reason for hiding this comment

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

Maybe. And modifying changes is a side effect. Since every compare method returns void, I'll rewrite them to return changes instead.

@bonndan bonndan merged commit 7f49e77 into develop Mar 28, 2021
@bonndan bonndan deleted the 436_processing_changes branch March 28, 2021 18:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants