search

dynamicdan / sn-filesync

2-way sync ServiceNow field values to local files
MIT License
66 stars 37 forks source link

Develop #35

Open geekLayla opened 7 years ago

geekLayla commented 7 years ago

@dynamicdan First of all thank you for this usefull tool.

I would like to inform you that I am working in a windows environnement and i can confirm "Home dir config" and "Exporting current setup" are working fine in windows.

Actually i am trying to implement the conflict management when pushing a file to serviceNow instance.

Could you please provide me with information about if any one has began working on that before?

Thanks you.

PS: This PR is just to inform you about my intention, it does not contain conflicts management code yet.

dynamicdan commented 7 years ago

@geekLayla, thanks for investing some time and letting me know what works on windows. Conflict checking should already be working (since the start). It's not possible to send a record update to the instance if there was a change done in-between.

The only thing we don't do is offer conflict resolution. Eg, downloading the change and opening a local diff tool to show the user changes to make a manual (or auto) merge. Would you like to work on that? I don't know the diff world in the windows OS.

geekLayla commented 7 years ago

Hello @dynamicdan I performed some changes in order to handle conflicts management feature.

Would you like to take a look at these changes and give me your feedback?

Thank you :)

dynamicdan commented 7 years ago

@geekLayla , sorry for the delay. I've been caught up in other things and needed to push some outstanding code to sn-filesync before looking into your PR.

Firstly, I'm impressed. I didn't think you'd integrate a full blown diff solution. I haven't tried it yet but I think it would be very useful.

My initial feedback is that we should avoid bloating the app.js file further. I really want to cut this down. Do you think we can modularise the diff logic into a seperate module? My idea with diff is that the user should be able to specify a diff app to use on their computer and a command in their config.json file to trigger it. We could then use your logic as a default if no diff app is specified. I see this as an add-on that can be included with SN-FS.

From the code I'm not sure if your solution can push up a merged diff. How would this work? Are you taking care of edge cases where another change has occurred before the merged version is pushed? Or does your solution assume that the merged version is safe to override the existing instance version?

I'm looking forward to your reply!

dynamicdan commented 7 years ago

@geekLayla do you have any feedback to my comments? I'd like to bring in your functionality but it needs some more work.