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

GCI Task: [netjsonconfig] TypeError: can only concatenate list (not "str") to list #124

Merged
merged 1 commit into from
Nov 27, 2018

Conversation

yasharora102
Copy link
Contributor

@yasharora102 yasharora102 commented Nov 25, 2018

This Issue is in reference to the issue #118 . It would be great to have your views on the changes made.
Thank you in advance.

Closes #118

@coveralls
Copy link

coveralls commented Nov 25, 2018

Coverage Status

Coverage increased (+0.0002%) to 99.931% when pulling be7fb2b on stealthflame:issue118 into 3d25204 on openwisp:master.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Your test is not being triggered, see my comment below with the suggested solution.


config interface 'eth0'
option ifname 'eth0'
list ipaddr 'fd87::1'
Copy link
Member

Choose a reason for hiding this comment

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

on OpenWRT, ipaddr is for ipv4, you have to use ip6addr to trigger your test, use the following:

option ip6addr '2aa1:4aaa:2aaa:1d::5/64'

Update the expected value accordingly as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @nemesisdesign for the feedback. I have made the changes and updated the PR 😄

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Good @stealthflame, now squash the commits into one and discard the second commit message

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Write a good commit message that follows our guidelines please.

@yasharora102
Copy link
Contributor Author

Done @nemesisdesign

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

Commit message still doesn't adhere to our guidelines. You need to learn this.
http://openwisp.io/docs/developer/contributing.html#commit-message-style-guidelines
Look at how other commits are structured and follow the same conventions.

@nemesifier
Copy link
Member

@stealthflame you don't need to close it, just push force to your branch

@yasharora102 yasharora102 reopened this Nov 27, 2018
@yasharora102
Copy link
Contributor Author

yasharora102 commented Nov 27, 2018

Done @nemesisdesign sorry for the inconvenience. I was editing my previous commit and by mistake made reset by hard.

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

@stealthflame you should have mentioned and closed the issue in the commit description as per our guidelines but I'm tired to send the task back to you. We are introducing an automatic check soon and this kind of errors will be automatically rejected by our build system so I suggest you to spend a bit more time understanding how to properly format your commit messages

@nemesifier nemesifier merged commit febebbc into openwisp:master Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants