Re-enable model::sync

[simoneromano96]

Solves #51

[thedodd]

@simoneromano96 awesome! I'll give this PR an in-depth review ASAP!

[thedodd]

@simoneromano96 just a quick heads-up: looking through the code it looks like there is a fair number of stylistic changes, which were a bit hard to follow at first, in terms of knowing if they were pertinent to the focus of the PR.

No big. I'm hacking on a PR right now to get some rustfmt config in place. Once that has landed, you should be able to rebase, and then the style changes should disappear.

[simoneromano96]

@simoneromano96 just a quick heads-up: looking through the code it looks like there is a fair number of stylistic changes, which were a bit hard to follow at first, in terms of knowing if they were pertinent to the focus of the PR.

No big. I'm hacking on a PR right now to get some rustfmt config in place. Once that has landed, you should be able to rebase, and then the style changes should disappear.

Hi, sorry for that, I ran the format document command from vscode via rust analyzer, I guess that did something

[thedodd]

Hey @simoneromano96 just wanted to circle back with you on this. I got the rustfmt code merged & the CI updated a bit. When you have the time, I would definitely appreciate it if you would be willing to rebase your branch. Now that rustfmt is in place, we shouldn't run into issues like this any longer.

Also, I strongly recommend that you squash all of your commits down to one logical commit before attempting to rebase ... otherwise the rebase will not be a good experience :)

Cheers

[simoneromano96]

Hey @simoneromano96 just wanted to circle back with you on this. I got the rustfmt code merged & the CI updated a bit. When you have the time, I would definitely appreciate it if you would be willing to rebase your branch. Now that rustfmt is in place, we shouldn't run into issues like this any longer.

Also, I strongly recommend that you squash all of your commits down to one logical commit before attempting to rebase ... otherwise the rebase will not be a good experience :)

Cheers

Hi, I noticed that I had to run cargo fmt with a stable toolchain but in the rustfmt.toml it uses some unstable features available on nightly, the CI and cargo fmt is launching some warnings.

I guess there has been some typos, I'll look at them now.

Warning: Unknown configuration option `chain_width`
Warning: Unknown configuration option `single_line_if_else_max_width`
Warning: Unknown configuration option `space_around_attr_eq`
Warning: Unknown configuration option `struct_lit_width`
Warning: Unknown configuration option `chain_width`
Warning: Unknown configuration option `single_line_if_else_max_width`
Warning: Unknown configuration option `space_around_attr_eq`
Warning: Unknown configuration option `struct_lit_width`
Warning: Unknown configuration option `chain_width`
Warning: Unknown configuration option `single_line_if_else_max_width`
Warning: Unknown configuration option `space_around_attr_eq`
Warning: Unknown configuration option `struct_lit_width`
Warning: Unknown configuration option `chain_width`
Warning: Unknown configuration option `single_line_if_else_max_width`
Warning: Unknown configuration option `space_around_attr_eq`
Warning: Unknown configuration option `struct_lit_width`

[thedodd]

@simoneromano96 yea, I've left them there because they are lints which I want to take place once they stabilize haha. Don't worry about them :) Good eye though.

[thedodd]

I suppose if folks find it super annoying, we can just comment them out. Though I'd prefer not to. Especially given that typically no one will see the warnings unless they are running rustfmt by hand.

[simoneromano96]

I suppose if folks find it super annoying, we can just comment them out. Though I'd prefer not to. Especially given that typically no one will see the warnings unless they are running rustfmt by hand.

Ok, no problem then, I tipically use the nightly toolchain so It had some inconsistencies but that's not a issue, we can go on with this PR, don't you think?

[thedodd]

@simoneromano96 one request that I have for you before I dive in further: do you mind doing git rebase -i HEAD~28 and squashing these down to one logical commit. Preferably following the standards in https://www.conventionalcommits.org/en/v1.0.0/ (don't worry about the type & scope in the opening line). In simple terms:

• please squash down to 1 commit
• please have the first line of the commit as a concise description of what is being done (~50 chars), followed by a blank line, then a more in depth description of the set of changes.

No need to go over the top; however, if you look at the commit history currently in the repo, it is clear and concise so far. Sorry for the runaround, mate. Cheers

[simoneromano96]

It should be ok now, I used the command git rebase -i HEAD~28 while on my fork's master, can you check that everything's ok? It had a good number of conflicts that I had to resolve.

[thedodd]

@simoneromano96 hey boss. Sorry for the late follow-up here. Looks like the code has conflicts. It looks like you will need to pull the latest from this upstream repo.

A few things I would recommend. Whenever you fork a repo and start doing new work, create a branch first. Don't commit directly to master on your branch. This will save you a lot of headaches long-term.

In this case, you should be able to just get the updates by doing git pull --rebase upstream master. The spec for the command: git pull [<options>] [<repository> [<refspec>...]], so in this case I am using upstream to refer to this repo, not your local repo (obvs).

You will quite likely have conflicts when you do this. If you are not comfortable doing a rebase, you can always just do git pull upstream master which will default to creating an update commit. Whatever you are comfortable with.

Once that bit is cleaned up, then I will be able to do the pre-merge review. There are a few changes that came to mind already, but let's get this knocked out first.

[simoneromano96]

@simoneromano96 hey boss. Sorry for the late follow-up here. Looks like the code has conflicts. It looks like you will need to pull the latest from this upstream repo.

A few things I would recommend. Whenever you fork a repo and start doing new work, create a branch first. Don't commit directly to master on your branch. This will save you a lot of headaches long-term.

In this case, you should be able to just get the updates by doing git pull --rebase upstream master. The spec for the command: git pull [<options>] [<repository> [<refspec>...]], so in this case I am using upstream to refer to this repo, not your local repo (obvs).

You will quite likely have conflicts when you do this. If you are not comfortable doing a rebase, you can always just do git pull upstream master which will default to creating an update commit. Whatever you are comfortable with.

Once that bit is cleaned up, then I will be able to do the pre-merge review. There are a few changes that came to mind already, but let's get this knocked out first.

Thanks for the advice, being my first PR everything is helpful, I'll try to fix everything this evening.

[simoneromano96]

Hello, sorry for the silence, the fork is now updated

[ndelvalle]

Any news on this PR? Sorry to bother, but I am not sure if waiting or implement my index management using another lang.

[thedodd]

@simoneromano96 thanks for the update! I'll put it on my shortlist to get this reviewed and merged. Just in the name of time optimization, if I find any issues, I'll just go ahead and merge and then change after it is merged :)

[MCOfficer]

I hate to be that guy, but any news on this PR?

[simoneromano96]

If I need to do anything else, just tell me!