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.

Sign up for GitHub

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

Jump to bottom

✨ bidirectional sync #5

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

✨ bidirectional sync #5

wants to merge 7 commits into from

Conversation

MB901
Copy link

@MB901 MB901 commented Jan 20, 2025

Hi, thank you for this addon!

I’ve made some improvements to enhance functionality and usability:

  • Added the ability to sync files from the remote server to Home Assistant (bidirectional sync).
  • Removed the unused remote_folder variable.
  • Updated folder naming conventions: switched from source/destination to local/remote to better reflect the purpose when using pull/push options.

Testing:

  • Verified the sync process in both directions (local-to-remote and remote-to-local).
  • Ensured that removing the remote_folder variable does not break existing configurations.

Let me know if you have any questions or need further changes.

Thanks!

github-actions[bot]
github-actions bot requested changes Jan 20, 2025
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['breaking-change', 'bugfix', 'documentation', 'enhancement', 'refactor', 'new-feature', 'maintenance', 'dependencies']

@MB901 MB901 changed the title Add feature: bidirectional sync new-feature: bidirectional sync Jan 20, 2025
@Poeschl Poeschl added the new-feature New features or options. label Jan 22, 2025
@Poeschl Poeschl changed the title new-feature: bidirectional sync ✨ bidirectional sync Jan 22, 2025
Poeschl
Poeschl requested changes Jan 22, 2025
View reviewed changes
Copy link
Member

@Poeschl Poeschl left a comment

Choose a reason for hiding this comment

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

Some minor comments from my side ;)

And it seems the linter picks up the old config folder. It's optional to fix that. If its not done in your PR, I will do it before release.

rsync/config.yaml Outdated Show resolved Hide resolved
rsync/root/run.sh
-e "ssh -p ${PORT} -i ${PRIVATE_KEY_FILE} -oStrictHostKeyChecking=no" \
"$local" "${USERNAME}@${HOST}:${remote}"
set +x
direction=$(echo "$FOLDERS" | jq -r ".[$i].direction // \"push\"")
Copy link
Member

Choose a reason for hiding this comment

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

It the direction should be optional, then a default value is needed here.

Copy link
Author

Choose a reason for hiding this comment

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

All other changes are done :)
but I don't understand this one...
"push" is used as the fallback value when no direction is defined in the configuration.

@MB901 MB901 requested a review from Poeschl January 29, 2025 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

@Poeschl Poeschl Awaiting requested review from Poeschl

Assignees
No one assigned
Labels
new-feature New features or options.
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MB901 @Poeschl