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

fix: Remove collection name from schema ID generation #1920

Conversation

AndrewSisley
Copy link
Contributor

Relevant issue(s)

Resolves #1083

Description

Removes collection name from schema ID generation.

It is a schema ID, not a collection ID, including the collection name makes no sense and is likely a historical artefact.

It is a schema ID, not a collection ID, inculding the collection name makes no sense and is likely a historical artifact.
@AndrewSisley AndrewSisley added bug Something isn't working area/schema Related to the schema system labels Oct 2, 2023
@AndrewSisley AndrewSisley requested a review from a team October 2, 2023 15:19
@AndrewSisley AndrewSisley self-assigned this Oct 2, 2023
@AndrewSisley AndrewSisley changed the title fix(i): Remove collection name from schema ID generation fix: Remove collection name from schema ID generation Oct 2, 2023
Copy link
Collaborator

@fredcarle fredcarle left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for removing the collection name in the generation!

@codecov
Copy link

codecov bot commented Oct 2, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (e952be5) 70.80% compared to head (11f735c) 70.73%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1920      +/-   ##
===========================================
- Coverage    70.80%   70.73%   -0.07%     
===========================================
  Files          233      233              
  Lines        24334    24331       -3     
===========================================
- Hits         17229    17210      -19     
- Misses        5932     5943      +11     
- Partials      1173     1178       +5     
Flag Coverage Δ
all-tests 70.73% <100.00%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
db/collection.go 72.98% <100.00%> (-0.08%) ⬇️

... and 5 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e952be5...11f735c. Read the comment docs.

@AndrewSisley AndrewSisley merged commit 6244c5b into sourcenetwork:develop Oct 2, 2023
14 checks passed
@AndrewSisley AndrewSisley deleted the 1083-rm-col-name-from-schema-id branch October 2, 2023 16:14
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
…1920)

## Relevant issue(s)

Resolves sourcenetwork#1083

## Description

Removes collection name from schema ID generation.

It is a schema ID, not a collection ID, including the collection name
makes no sense and is likely a historical artefact.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/schema Related to the schema system bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make schemaID only dependant on the schema description.
2 participants