@ncw Someone else posted on my pull request, I made most of the fixes they mentioned but I was unsure about the ListFn
.
This was from your post further up, where should that be declared, they mentioned the following.
Maybe this needs to be operations.ListFn..., and with an "github.com/rclone/rclone/fs/operations " entry in import up in the beginning of the file.
rclone:master
← aeromaxx:master
opened 01:08PM - 06 May 21 UTC
<!--
Thank you very much for contributing code or documentation to rclone! Plea… se
fill out the following questions to make it easier for us to review your
changes.
You do not need to check all the boxes below all at once, feel free to take
your time and add more commits. If you're done and ready for review, please
check the last box.
-->
#### What is the purpose of this change?
<!--
Add --recursive flag to rclone touch command
-->
#### Was the change discussed in an issue or in the forum before?
<!--
https://forum.rclone.org/t/rclone-touch-recursive/24015
https://github.com/rclone/rclone/issues/5301
-->
#### Checklist
- [ ] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-pull-request).
- [x] I have added tests for all changes in this PR if appropriate.
- [ ] I have added documentation for the changes if appropriate.
- [ ] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [ ] I'm done, this Pull Request is ready for review :-)
ncw
(Nick Craig-Wood)
May 10, 2021, 4:26pm
22
Yes that will fix it up if you want to have a go.
However I'd really like the whole TouchRecursive
function , plus tests in fs/operations/operations.go
rather than in cmd/touch/touch.go
and I'd like it to take a time.Time
as the parameter.
@ncw I'm confused as why it's fine for the Touch function to be in touch.go, but not for the TouchRecursive function? Also the TouchRecursive function does take a time.Time parameter.
Are you asking for new files such as ListFn.go
to be in operations
, and that file would contain this maybe?
func ListFn(ctx, f, func(o fs.Object) {
err := o.SetModTime(ctx, timeAtr)
if err != nil {
err = fs.CountError(err)
fs.Errorf(o, "touch: couldn't set mod time %v", err)
}
}
ncw
(Nick Craig-Wood)
May 11, 2021, 3:34pm
24
It is to do with testing. Any code in operations will get tested against all the backends in the integration tests. Whereas the code in cmd
doesn't.
operations
has a ListFn already and that function just does SetModTime
on a single file which is already tested so doesn't need to go in operations
.
Its not a big deal - you can leave TouchRecursive
in cmd/touch
as its building blocks SetModTime
and ListFn
are well tested against all backends.
@ncw I have fixed up a few things, no new pull request yet, I can't get Go environment to work on Debian. I made a separate post about this.
@ncw What does this mean?
imports github.com/rclone/rclone/fs/operations: import cycle not allowed
Perhaps you can update your pull request, even if your code does not build. Easier to help when we see the full source code..
@albertony How do I update a pull request? not sure how to do that, I was just going to submit a new pull request.
I see your pull request is from your master. Normally one creates the pull request from a branch, and then one can just commit and push additional changes and they will be included in the pull request automatically. Not sure with master, but maybe its the same...
Oh I deleted my old repo, and created a new repo and have a new branch on that one.
OK. I guess you can just create a new pull request as you planned, then.
@ncw @albertony This is the new pull request.
rclone:master
← aeromaxx:rclone-touch-recursive
opened 09:30PM - 14 May 21 UTC
<!--
Thank you very much for contributing code or documentation to rclone! Plea… se
fill out the following questions to make it easier for us to review your
changes.
You do not need to check all the boxes below all at once, feel free to take
your time and add more commits. If you're done and ready for review, please
check the last box.
-->
#### What is the purpose of this change?
<!--
Add --recursive flag to rclone touch command
-->
#### Was the change discussed in an issue or in the forum before?
<!--
https://forum.rclone.org/t/rclone-touch-recursive/24015
https://github.com/rclone/rclone/issues/5301
-->
#### Checklist
- [x] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-pull-request).
- [x] I have added tests for all changes in this PR if appropriate.
- [x] I have added documentation for the changes if appropriate.
- [x] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [ ] I'm done, this Pull Request is ready for review :-)
Great my new pull request has failed again, with errors that don't tell you why it failed!
If you click details on the failing checks, you can go into the build log and you will see the errors.
E.g line 313 on windows 386:
fs\operations\touch.go:5:2: imported and not used: "fmt"
Where did you find that?
All I can see is...
Error: make: *** [Makefile:49: rclone] Error 2
Error: Process completed with exit code 2.
rclone:master
← aeromaxx:rclone-touch-recursive
opened 09:30PM - 14 May 21 UTC
<!--
Thank you very much for contributing code or documentation to rclone! Plea… se
fill out the following questions to make it easier for us to review your
changes.
You do not need to check all the boxes below all at once, feel free to take
your time and add more commits. If you're done and ready for review, please
check the last box.
-->
#### What is the purpose of this change?
<!--
Add --recursive flag to rclone touch command
-->
#### Was the change discussed in an issue or in the forum before?
<!--
https://forum.rclone.org/t/rclone-touch-recursive/24015
https://github.com/rclone/rclone/issues/5301
-->
#### Checklist
- [x] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-pull-request).
- [x] I have added tests for all changes in this PR if appropriate.
- [x] I have added documentation for the changes if appropriate.
- [x] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [ ] I'm done, this Pull Request is ready for review :-)
I don't fully understand what is the difference here... But first of all you must be logged in to GitHub. Then, either click Details on the failing checks, or go into the Actions tab and click your failing workflow. Then you must click on the individual jobs in the left pane, e.g. "windows_amd64", and you should see the full build log to the right. ...or maybe this is a permission thing, so you have not access to this?
This is what I see:
Ole
(Ole Frost)
May 15, 2021, 10:54am
37
I haven't read all the details, but may be this tip was more applicable in this thread
I saw that Good advice in your last post there!
Ole
(Ole Frost)
May 15, 2021, 11:48am
39
You may find guidance in my draft update of the rclone contributing guide .
Please let me know if you find anything wrong or missing, while preparing your recursive touch.
The above draft is my first iteration on this:
rclone:master
← olefrost:contributing-improved-for-git-beginners
opened 01:49PM - 04 May 21 UTC
#### What is the purpose of this change?
To make it easier to contribute with… out prior knowledge of Git, GitHub and Go.
The change includes improved/added steps to:
* Install Git with basic setup
* Use both SSH and HTTPS for the git origin
* Install Go and verify the GOPATH
* Update the forked master
* Find a popular editor for Go
#### Was the change discussed in an issue or in the forum before?
The addition of an HTTPS link for the git origin was discussed and approved in this forum thread:
https://forum.rclone.org/t/contributing-guide-git-github-com-https-github-com/23855
The rest is based on my own experiences while preparing [my first contribution](https://github.com/rclone/rclone/pull/5273) on a Windows computer.
I have done my best to research the changes and make the guide applicable for all platforms, protocols and preferences.
I am however still a beginner, so a careful review by somebody with native skills in Go, Git, GitHub and non-Windows platforms is needed.
#### Checklist
- [x] I have read the [contribution guidelines](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#submitting-a-pull-request).
- [x] I have tested the added/modified links.
- [x] I have tested the modified parts of the installation flow on Windows.
- [x] I have tested the document layout in Preview mode on GitHub and Visual Studio Code.
- [x] All commit messages are in [house style](https://github.com/rclone/rclone/blob/master/CONTRIBUTING.md#commit-messages).
- [x] I'm done, this Pull Request is ready for review :-)
system
(system)
Closed
May 18, 2021, 11:49am
40
This topic was automatically closed 3 days after the last reply. New replies are no longer allowed.