sync new changes in zos3 #11
Conversation
Omarabdul3ziz
requested review from
muhamadazmy,
ashraffouda,
rawdaGastan,
xmonader and
Eslam-Nawara
as code owners
There was a problem hiding this comment.
Is this to bring fixes from zos3 to zos4 ? you probably need to find a better way to do this in the future. From what I see it looks like zos3 is not going anywhere anytime soon.
With careful design, maybe the zos3 code base can be better separated into smaller reusable packages. Which can be used by both zos3 and 4.
Only specific version code can be different.
Of course this is easier said than done.
| @@ -146,6 +152,10 @@ func (r *Registrar) register(ctx context.Context, cl zbus.Client, env environmen | |||
| if err := r.reActivate(ctx, cl, env); err != nil { | |||
| log.Error().Err(err).Msg("failed to reactivate account") | |||
| } | |||
| case <-time.After(updateLocationInterval): | |||
There was a problem hiding this comment.
I thought we going to just call register() here which will also update the locaiton
There was a problem hiding this comment.
i did on the zos3 pr threefoldtech@7e0ee7d was waiting for an approve there to cherry-pick the commit here. done now
|
yes, it is not the best way to do it i was discussing with Ashraf about it. a suggestion for the coming changes was to open two PRs from the same branch against the two repos. but indeed having reusable packages is a cleaner option |
Yes, i do have the same concerns about this double repos to Ashraf & Jan.
I argue that it is not "is not the best way to do", but it is a bad way to do it.
Yes, we should do it. |
- add updater package - expose GetState from registerer - fix longitude spelling
- add updater package - expose GetState from registerer - fix longitude spelling
- add updater package - expose GetState from registerer - fix longitude spelling
- add updater package - expose GetState from registerer - fix longitude spelling
Omarabdul3ziz
force-pushed
the
main_zos3_sync
branch
from
334621e
to
db6295a
Compare
|
Related Prs on zos repo: