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

Rename service id to hash #626

Closed
wants to merge 16 commits into from
Closed

Conversation

NicolasMahe
Copy link
Member

@NicolasMahe NicolasMahe commented Dec 6, 2018

Depend on #627
Second half of #625

@NicolasMahe NicolasMahe self-assigned this Dec 6, 2018
@NicolasMahe NicolasMahe changed the base branch from dev to feature/rename-alias-to-sid December 6, 2018 13:58
@NicolasMahe NicolasMahe changed the title [WIP] Rename service id and alias [WIP] Rename service id to hash Dec 6, 2018
@NicolasMahe
Copy link
Member Author

NicolasMahe commented Dec 6, 2018

This PR introduces one change on the api:
https://github.com/mesg-foundation/core/pull/626/files#diff-3ec4078a2b83ce9f8a52b41a8f1f4397R476

-  string ID = 10;                           // Service's unique id service hash.
+  string Hash = 10;                           // Service's unique id service hash.

@ilgooz @krhubert I wonder if it's a breaking change. The type, and the number are the same. Only the name change. What do you think?

@NicolasMahe NicolasMahe changed the title [WIP] Rename service id to hash Rename service id to hash Dec 6, 2018
@ilgooz
Copy link
Contributor

ilgooz commented Dec 6, 2018

This PR introduces one change on the api:
https://github.com/mesg-foundation/core/pull/626/files#diff-3ec4078a2b83ce9f8a52b41a8f1f4397R476

-  string ID = 10;                           // Service's unique id service hash.
+  string Hash = 10;                           // Service's unique id service hash.

@ilgooz @krhubert I wonder if it's a breaking change. The type, and the number are the same. Only the name change. What do you think?

It shouldn't be a breaking change. Clients just will keep using ID key. But for the clients that directly imports the pb file from protobbuf pkg, it'll be a breaking change.

database/service_db.go Outdated Show resolved Hide resolved
protobuf/coreapi/api.proto Outdated Show resolved Hide resolved
service/service.go Outdated Show resolved Hide resolved
database/service_db.go Outdated Show resolved Hide resolved
database/service_db_test.go Outdated Show resolved Hide resolved
database/service_db_test.go Outdated Show resolved Hide resolved
database/service_db_test.go Outdated Show resolved Hide resolved
database/service_db_test.go Outdated Show resolved Hide resolved
database/service_db_test.go Outdated Show resolved Hide resolved
database/service_db_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

Some changes are mandatory.

Copy link
Contributor

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

Some changes are mandatory.

NicolasMahe and others added 6 commits December 7, 2018 10:29
…e-ID-and-alias

# Conflicts:
#	database/service_db.go
#	database/service_db_test.go
#	protobuf/coreapi/api.pb.go
#	protobuf/coreapi/api.proto
#	service/importer/assets/schema.go
Co-Authored-By: NicolasMahe <nicolas@mahe.me>
api/execute.go Show resolved Hide resolved
database/service_db.go Outdated Show resolved Hide resolved
database/service_db.go Outdated Show resolved Hide resolved
protobuf/coreapi/api.proto Outdated Show resolved Hide resolved
Copy link
Contributor

@ilgooz ilgooz left a comment

Choose a reason for hiding this comment

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

Needs some changes

@ilgooz
Copy link
Contributor

ilgooz commented Dec 7, 2018

Need to re-gen proto

@NicolasMahe NicolasMahe changed the base branch from feature/rename-alias-to-sid to dev December 7, 2018 07:19
@NicolasMahe
Copy link
Member Author

Close this PR in favor of #630

@NicolasMahe NicolasMahe closed this Dec 7, 2018
@NicolasMahe NicolasMahe deleted the feature/rename-service-ID-and-alias branch December 7, 2018 08:05
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.

2 participants