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.

Sign up for GitHub

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

Jump to bottom

Swarm filters save config (fix #2878) #2880

Merged
merged 4 commits into from
Jun 21, 2016
Merged

Swarm filters save config (fix #2878) #2880

merged 4 commits into from
Jun 21, 2016

Conversation

yuvallanger
Copy link
Contributor

@yuvallanger yuvallanger commented Jun 20, 2016 •
edited
Loading

Please review. (ref: #2878)

@yuvallanger
Add filtersAdd() and the code that uses it to swarmFiltersAddCmd.Run()
bd9b312 
License: MIT
Signed-off-by: Yuval Langer <yuval.langer@gmail.com>
@yuvallanger
Add filtersRemove() and the swarmFiltersRmCmd.Run() parts that call it.
42468d0 
Also added types and marshalers to the swarmFiltersAddCmd and
swarmFiltersRmCmd structs.

License: MIT
Signed-off-by: Yuval Langer <yuval.langer@gmail.com>
Kubuxu
Kubuxu reviewed Jun 20, 2016
View reviewed changes
core/commands/swarm.go

if len(req.Arguments()) == 0 {
res.SetError(errors.New("no filters to add"), cmds.ErrClient)
return
Copy link
Member

Choose a reason for hiding this comment

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

Move this before opening the repo

@yuvallanger
Move no arguments check to before opening fsrepo.
20b6499 
License: MIT
Signed-off-by: Yuval Langer <yuval.langer@gmail.com>
@Kubuxu
Copy link
Member

Kubuxu commented Jun 20, 2016

To prevent future regressions it would be great to have test cases for this behaviour, please check test/sharness/t0141-addfilter.sh and write testcase that will be testing fixed behaviour.

Apart from that it looks quite good.

@yuvallanger
Copy link
Contributor Author

Added tests to test/sharness/t0141-addfilter.sh.
@Kubuxu LFCR

@yuvallanger
Add ipfs config Swarm.AddrFilters test
d608f32 
License: MIT
Signed-off-by: Yuval Langer <yuval.langer@gmail.com>
@yuvallanger
Copy link
Contributor Author

yuvallanger commented Jun 21, 2016 •
edited
Loading

touch does not truncates an existing file. Filter addresses are only added to the list_expected file with each call to test_config_swarm_addrfilters_cmd. printf "" > list_expected works pretty well in this case.

@Kubuxu
Copy link
Member

Kubuxu commented Jun 21, 2016

LGTM

@Kubuxu Kubuxu added the RFM label Jun 21, 2016
@whyrusleeping
Copy link
Member

LGTM, thanks @yuvallanger

@whyrusleeping whyrusleeping merged commit 7d18f74 into ipfs:master Jun 21, 2016
@yuvallanger yuvallanger deleted the swarm-filters-save-config branch September 3, 2017 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
RFM
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yuvallanger @Kubuxu @whyrusleeping