TrueCloudLab
/
frostfs-node
18
1
Fork 20
Code Issues 95 Pull Requests 7 Actions Releases Activity

Drop/resolve TODO's #847

Closed
dstepanov-yadro wants to merge 1 commits from dstepanov-yadro/frostfs-node:fix/todo into master
pull from: dstepanov-yadro/frostfs-node:fix/todo
merge into: TrueCloudLab:master
Conversation 1 Commits 1 Files Changed 30
dstepanov-yadro commented 2023-12-06 13:29:17 +00:00
Collaborator

The most of TODOs are more than one year old, so looks like they will be never resolved.

Some "young" TODOs are still present.

The most of TODOs are more than one year old, so looks like they will be never resolved. Some "young" TODOs are still present.
dstepanov-yadro force-pushed fix/todo from 48efdb391a to 940554edce 2023-12-06 13:30:44 +00:00 Compare
dstepanov-yadro requested review from storage-core-committers 2023-12-06 13:31:04 +00:00
dstepanov-yadro requested review from storage-core-developers 2023-12-06 13:31:09 +00:00
dstepanov-yadro force-pushed fix/todo from 940554edce to 63a87eb3c5 2023-12-06 13:32:10 +00:00 Compare
dstepanov-yadro force-pushed fix/todo from 63a87eb3c5 to d4be3ea940 2023-12-06 13:32:55 +00:00 Compare
fyrchik reviewed 2023-12-06 13:35:42 +00:00
pkg/local_object_storage/engine/evacuate.go Outdated
@ -276,4 +276,2 @@
for {
listPrm.WithCursor(c)
// TODO (@fyrchik): #1731 this approach doesn't work in degraded modes
fyrchik commented 2023-12-06 13:34:30 +00:00
Owner

Please, let's leave this TODO as it is, it is sth we would like to support.

Please, let's leave this TODO as it is, it is sth we would like to support.
dstepanov-yadro commented 2023-12-06 13:47:08 +00:00
Poster
Collaborator

#848

https://git.frostfs.info/TrueCloudLab/frostfs-node/issues/848
fyrchik commented 2023-12-06 16:00:47 +00:00
Owner

What is the problem with still having it in code?

What is the problem with still having it in code?
dstepanov-yadro commented 2023-12-07 06:02:51 +00:00
Poster
Collaborator

The task can be scheduled and completed. The comment in the code will be lost.

The task can be scheduled and completed. The comment in the code will be lost.
fyrchik commented 2023-12-07 06:59:07 +00:00
Owner

The comment reminds you that the task exists in the first place, so everyone who touches the code sees possible implications.

The comment reminds you that the task exists in the first place, so everyone who touches the code sees possible implications.
dstepanov-yadro commented 2023-12-07 08:43:21 +00:00
Poster
Collaborator

Ok, reverted comment.

Ok, reverted comment.
fyrchik commented 2023-12-07 11:25:44 +00:00
Owner

It is different now and misses the link to the task.

It is different now and misses the link to the task.
dstepanov-yadro commented 2023-12-07 12:59:54 +00:00
Poster
Collaborator

The meaning of the comment is the same. Why store references to tasks in the code?

The meaning of the comment is the same. Why store references to tasks in the code?
fyrchik commented 2023-12-07 14:23:50 +00:00
Owner

To link to the task and discussion (and not to create it again).

To link to the task and discussion (and not to create it again).
dstepanov-yadro commented 2023-12-07 17:11:28 +00:00
Poster
Collaborator

Source code is not the best place for discussions:)

Then we need to add links to all tasks from gitea to the source code.

Source code is not the best place for discussions:) Then we need to add links to all tasks from gitea to the source code.
fyrchik commented 2023-12-08 07:36:13 +00:00
Owner

Source code is not the best place for discussions:)

Exactly, and link links to a proper place for discussion.

Then we need to add links to all tasks from gitea to the source code.

No, we don't. In some cases we feel the need to leave a comment, nothing wrong with it.

>Source code is not the best place for discussions:) Exactly, and link links to a proper place for discussion. >Then we need to add links to all tasks from gitea to the source code. No, we don't. In _some_ cases we feel the need to leave a comment, nothing wrong with it.
pkg/services/object/get/container.go Outdated
@ -79,4 +79,2 @@
// TODO: #1142 consider parallel execution
// TODO: #1142 consider optimization: if status == SPLIT we can continue until
// we reach the best result - split info with linking object ID.
fyrchik commented 2023-12-06 13:35:15 +00:00
Owner

This is a continuation of TODO comment, could remove

This is a continuation of `TODO` comment, could remove
dstepanov-yadro commented 2023-12-06 13:47:59 +00:00
Poster
Collaborator

done

done
dstepanov-yadro force-pushed fix/todo from d4be3ea940 to 41e22f69e7 2023-12-06 13:47:54 +00:00 Compare
acid-ant reviewed 2023-12-07 06:50:02 +00:00
cmd/frostfs-cli/modules/object/get.go Outdated
@ -108,7 +108,6 @@ func getObject(cmd *cobra.Command, _ []string) {
func processResult(cmd *cobra.Command, res *internalclient.GetObjectRes, binary bool, payloadBuffer *bytes.Buffer, out io.Writer, filename string) {
if binary {
objToStore := res.Header()
// TODO(@acid-ant): #1932 Use streams to marshal/unmarshal payload
acid-ant commented 2023-12-07 06:47:21 +00:00
Collaborator

Could you please replace it on #849?

Could you please replace it on #849?
dstepanov-yadro commented 2023-12-07 08:44:51 +00:00
Poster
Collaborator

Why do I need a link to the task in the code? I think it is possible to delete this TODO, especially if there is a task.

Why do I need a link to the task in the code? I think it is possible to delete this TODO, especially if there is a task.
👍 1
acid-ant commented 2023-12-07 09:17:17 +00:00
Collaborator

Yeah, agree.

Yeah, agree.
cmd/frostfs-cli/modules/object/put.go Outdated
@ -156,7 +156,6 @@ func readFilePayload(filename string, cmd *cobra.Command) (io.Reader, cid.ID, us
buf, err := os.ReadFile(filename)
commonCmd.ExitOnErr(cmd, "unable to read given file: %w", err)
objTemp := objectSDK.New()
// TODO(@acid-ant): #1932 Use streams to marshal/unmarshal payload
acid-ant commented 2023-12-07 06:47:30 +00:00
Collaborator

Could you please replace it on #849?

Could you please replace it on #849?
dstepanov-yadro force-pushed fix/todo from 41e22f69e7 to cb6d097e0a 2023-12-07 08:42:59 +00:00 Compare
dstepanov-yadro commented 2023-12-08 11:13:19 +00:00
Poster
Collaborator

My position on TODO has not found support.

My position on TODO has not found support.
dstepanov-yadro closed this pull request 2023-12-08 11:13:20 +00:00
All checks were successful
DCO action / DCO (pull_request) Successful in 2m7s
Details
Vulncheck / Vulncheck (pull_request) Successful in 2m21s
Details
Build / Build Components (1.20) (pull_request) Successful in 3m43s
Details
Build / Build Components (1.21) (pull_request) Successful in 3m38s
Details
Tests and linters / Staticcheck (pull_request) Successful in 5m59s
Details
Tests and linters / Tests (1.20) (pull_request) Successful in 6m26s
Details
Tests and linters / Tests with -race (pull_request) Successful in 6m38s
Details
Tests and linters / Lint (pull_request) Successful in 6m58s
Details
Tests and linters / Tests (1.21) (pull_request) Successful in 3m8s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
TrueCloudLab/storage-core-committers
TrueCloudLab/storage-core-developers
Labels
Clear labels   
P0

Highest   
P1

High   
P2

Medium   
P3

Low   
badger

Badger DB related issues   
frostfs-adm
   
frostfs-cli
   
frostfs-ir
   
frostfs-lens
   
frostfs-node
   
good first issue

Good for newcomers   
triage
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff   
blocked

The issue or PR is blocked by something   
bug

Something is not working   
config

Configuration format update or breaking change   
discussion

Talking about something in order to reach a decision or to exchange ideas   
documentation

Documentation related   
duplicate

This issue or pull request already exists   
enhancement

New feature   
go

Language features adoption   
help wanted

Extra attention is needed   
internal
   
invalid

Something is wrong   
kludge

This is a kludge and we know it   
observability

Related to metrics, tracing and logs   
perfomance

Perfomance related   
question

More information is needed   
refactoring

Refactoring   
wontfix

This won't be fixed
No Label
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#847
There is no content yet.
Delete Branch "dstepanov-yadro/frostfs-node:fix/todo"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?