spring-cloud / spring-cloud-consul

Spring Cloud Consul
http://cloud.spring.io/spring-cloud-consul/
Apache License 2.0
813 stars 541 forks source link

Reinstate tags-as-metadata adding option to merge tags. Fixes #699 #700

Closed lucasoares closed 1 year ago

lucasoares commented 3 years ago

Hello.

See #699 and linked issues for more information.

This is my first PR into spring-cloud-consul. To be honest I didn't explore the project due to lack of time so I wanted to make the few changes as possible. Please let me know if there is anything to change.

I also didn't had time to configure checkstyle so I will wait for for CI results.

Just saw I authored commits using another user account. My bad.

Thanks.

lucasoares commented 3 years ago

If you look at 2a667f48e0dcb665886b85796597ff947f181bba055c1216d573b68cb3764a8c, f672845 and 6485240 there is some more to do to get back to parity (though merged rather than replaced).

Looking into it.

I will remove draft status when ready.

@spencergibb on f672845 we should set both tags and metadata if the merge is enabled? Because before we had a property for tags or metadata. Now we have a property for tags and metadata. What I should do?

spencergibb commented 3 years ago

Yes, I think it should be a merge.

spencergibb commented 3 years ago

@lucasoares any chance to update this?

lucasoares commented 3 years ago

@lucasoares any chance to update this?

Hello @spencergibb.

Unfortunately, I had no time to check it. I will try to get back to it this weekend.

lucasoares commented 1 year ago

Closing in favor of https://github.com/spring-cloud/spring-cloud-consul/pull/775