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

Develop #35

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Develop #35

wants to merge 13 commits into from

Conversation

geekLayla
Copy link

@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
Copy link
Owner

@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.

Copy link
Owner

@dynamicdan dynamicdan left a comment

Choose a reason for hiding this comment

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

You need to remove your access credentials!!!

@geekLayla
Copy link
Author

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
Copy link
Owner

@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
Copy link
Owner

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

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