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

Consistent underscoring, prepend non-argument types with (_) #2162

Open
shahzadlone opened this issue Jan 3, 2024 · 0 comments
Open

Consistent underscoring, prepend non-argument types with (_) #2162

shahzadlone opened this issue Jan 3, 2024 · 0 comments
Labels
code quality Related to improving code quality deprecate Indicates something is deprecated.

Comments

@shahzadlone
Copy link
Member

Part of #1750

Consistent underscoring, prepend non-argument types with (_)

While doing #1749, the issue of non-underscored docID/docIDs mixing with underscored _docID became more prominent. Consider:

query {
    User(docIDs: ["bae-6a6482a8-24e1-5c73-a237-ca569e41507d"]) {
        _docID
    }
}
query {
    Users {
        Name
        _docID
        _version {
            docID
        }
    }
}
Request: ` {
    commits(groupBy: [docID], order: {docID: DESC}) {
        docID
    }
}`,
Results: []map[string]any{
    {
        "docID": "bae-f54b9689-e06e-5e3a-89b3-f3aee8e64ca7",
    },
    {
        "docID": "bae-72f3dc53-1846-55d5-915c-28c4e83cc891",
    },
},

After the discussion the solution reached was to make all non-argument types _ underscored and leave the argument types without underscore. So the following would still remain valid:

query {
	User(docIDs: ["bae-6a6482a8-24e1-5c73-a237-ca569e41507d"]) {
		_docID
	}
}

So this issue is for the task to essentially add the underscores to all these from the SIP (https://github.com/sourcenetwork/SIPs/pull/15/files):
image

NOTE: dockey from above is now docID. So it will change to _docID after this issue.

@shahzadlone shahzadlone added code quality Related to improving code quality deprecate Indicates something is deprecated. labels Jan 3, 2024
shahzadlone added a commit that referenced this issue Jan 4, 2024
## Relevant issue(s)
- Subtask of #1750 [EPIC]
- [x] Resolves #1752
- [x] Resolves #1272


## BREAKING CHANGE:
- Use of `_key` to access a document's unique id is now deprecated,
instead use `_docID`.
- Use of `dockey`/`id` is now deprecated, instead use `docID`.
- Use of `dockeys`/`ids` is now deprecated, instead use `docIDs`.


## Description
- [x] Rename `_key` to `_docID` everywhere.
- [x] Rename `_keys` to `docIDs` in explain the response.
- [x] Rename `_newKey` to `_docIDNew` in backup/recover functionality.
- [x] Fix `_docID` tests.
- [x] Fix explain and backup/recover functionality tests.
- [x] Fix the collectionID order for a P2P test (leaving a note as order
was reverted).
- [x] Update all cids.
- [x] Rename all files with `key(s)|dockey(s)` in the name to
`doc_id(s)`.
- [x] Document breaking change to pass change detector.

## For Reviewers:
- Main commits to review are the `PR(MAIN)` commits. 
- If you have more time `PR(MINOR) and PR(*TEST)` commits are good to go
over


## Disclaimer / Discussion:
I do not like these non-underscored `docID/docIDs`, would be in favor of
underscoring these :

```
query {
    User(docIDs: ["bae-6a6482a8-24e1-5c73-a237-ca569e41507d"]) {
        _docID
    }
}
```

```
query {
    Users {
        Name
        _docID
        _version {
            docID
        }
    }
}
```

```
Request: ` {
    commits(groupBy: [docID], order: {docID: DESC}) {
        docID
    }
}`,
Results: []map[string]any{
    {
        "docID": "bae-f54b9689-e06e-5e3a-89b3-f3aee8e64ca7",
    },
    {
        "docID": "bae-72f3dc53-1846-55d5-915c-28c4e83cc891",
    },
},

```

EDIT: Above was resolved with #2162, to do out of this PR.


## Limitations (out of scope of this PR):
- #1467
- #1751
- #1550
- #2162
shahzadlone added a commit to shahzadlone/defradb that referenced this issue Jan 22, 2024
## Relevant issue(s)
- Subtask of sourcenetwork#1750 [EPIC]
- [x] Resolves sourcenetwork#1752
- [x] Resolves sourcenetwork#1272


## BREAKING CHANGE:
- Use of `_key` to access a document's unique id is now deprecated,
instead use `_docID`.
- Use of `dockey`/`id` is now deprecated, instead use `docID`.
- Use of `dockeys`/`ids` is now deprecated, instead use `docIDs`.


## Description
- [x] Rename `_key` to `_docID` everywhere.
- [x] Rename `_keys` to `docIDs` in explain the response.
- [x] Rename `_newKey` to `_docIDNew` in backup/recover functionality.
- [x] Fix `_docID` tests.
- [x] Fix explain and backup/recover functionality tests.
- [x] Fix the collectionID order for a P2P test (leaving a note as order
was reverted).
- [x] Update all cids.
- [x] Rename all files with `key(s)|dockey(s)` in the name to
`doc_id(s)`.
- [x] Document breaking change to pass change detector.

## For Reviewers:
- Main commits to review are the `PR(MAIN)` commits. 
- If you have more time `PR(MINOR) and PR(*TEST)` commits are good to go
over


## Disclaimer / Discussion:
I do not like these non-underscored `docID/docIDs`, would be in favor of
underscoring these :

```
query {
    User(docIDs: ["bae-6a6482a8-24e1-5c73-a237-ca569e41507d"]) {
        _docID
    }
}
```

```
query {
    Users {
        Name
        _docID
        _version {
            docID
        }
    }
}
```

```
Request: ` {
    commits(groupBy: [docID], order: {docID: DESC}) {
        docID
    }
}`,
Results: []map[string]any{
    {
        "docID": "bae-f54b9689-e06e-5e3a-89b3-f3aee8e64ca7",
    },
    {
        "docID": "bae-72f3dc53-1846-55d5-915c-28c4e83cc891",
    },
},

```

EDIT: Above was resolved with sourcenetwork#2162, to do out of this PR.


## Limitations (out of scope of this PR):
- sourcenetwork#1467
- sourcenetwork#1751
- sourcenetwork#1550
- sourcenetwork#2162
shahzadlone added a commit to shahzadlone/defradb that referenced this issue Feb 23, 2024
## Relevant issue(s)
- Subtask of sourcenetwork#1750 [EPIC]
- [x] Resolves sourcenetwork#1752
- [x] Resolves sourcenetwork#1272


## BREAKING CHANGE:
- Use of `_key` to access a document's unique id is now deprecated,
instead use `_docID`.
- Use of `dockey`/`id` is now deprecated, instead use `docID`.
- Use of `dockeys`/`ids` is now deprecated, instead use `docIDs`.


## Description
- [x] Rename `_key` to `_docID` everywhere.
- [x] Rename `_keys` to `docIDs` in explain the response.
- [x] Rename `_newKey` to `_docIDNew` in backup/recover functionality.
- [x] Fix `_docID` tests.
- [x] Fix explain and backup/recover functionality tests.
- [x] Fix the collectionID order for a P2P test (leaving a note as order
was reverted).
- [x] Update all cids.
- [x] Rename all files with `key(s)|dockey(s)` in the name to
`doc_id(s)`.
- [x] Document breaking change to pass change detector.

## For Reviewers:
- Main commits to review are the `PR(MAIN)` commits. 
- If you have more time `PR(MINOR) and PR(*TEST)` commits are good to go
over


## Disclaimer / Discussion:
I do not like these non-underscored `docID/docIDs`, would be in favor of
underscoring these :

```
query {
    User(docIDs: ["bae-6a6482a8-24e1-5c73-a237-ca569e41507d"]) {
        _docID
    }
}
```

```
query {
    Users {
        Name
        _docID
        _version {
            docID
        }
    }
}
```

```
Request: ` {
    commits(groupBy: [docID], order: {docID: DESC}) {
        docID
    }
}`,
Results: []map[string]any{
    {
        "docID": "bae-f54b9689-e06e-5e3a-89b3-f3aee8e64ca7",
    },
    {
        "docID": "bae-72f3dc53-1846-55d5-915c-28c4e83cc891",
    },
},

```

EDIT: Above was resolved with sourcenetwork#2162, to do out of this PR.


## Limitations (out of scope of this PR):
- sourcenetwork#1467
- sourcenetwork#1751
- sourcenetwork#1550
- sourcenetwork#2162
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Related to improving code quality deprecate Indicates something is deprecated.
Projects
None yet
Development

No branches or pull requests

1 participant