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

WIP: Implement new groupchats #1271

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
Open

WIP: Implement new groupchats #1271

wants to merge 10 commits into from

Conversation

jhert0
Copy link
Member

@jhert0 jhert0 commented Aug 8, 2018

This change is Reviewable

@jhert0 jhert0 force-pushed the new_groupchats branch 2 times, most recently from b079ea2 to a940466 Compare September 3, 2018 04:20
Comment on lines -77 to +80
snprintf((char *)g->name, sizeof(g->name), "Groupchat #%u", group_number);
g->name_length = strnlen(g->name, sizeof(g->name) - 1);
g->name_length = snprintf(g->name, sizeof(g->name), "Groupchat #%u", group_number);
if (g->name_length >= sizeof(g->name)) {
g->name_length = sizeof(g->name) - 1;
}
Copy link

Choose a reason for hiding this comment

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

Please don't change unrelated things without comment.

See d297e0d and #1346.

.style_toggle = BM_SWITCH_TOGGLE,
.style_icon_off = BM_NO,
.style_icon_on = BM_YES,
.switch_on = true, //private group by default
Copy link

Choose a reason for hiding this comment

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

Space after // here.

Comment on lines -28 to +30
edit_group_topic;
edit_group_topic,
edit_group_name;
Copy link

Choose a reason for hiding this comment

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

Small note: If you added the variable between chat_msg_group and group_topic, you would have not touched group_topic needlessly (git blame), and the diff would be more trivial.
And I think I actually prefer not to touch that line.

src/tox.c Outdated
* param2: audio call or not
* data: group name
*/
// TODO: readd checking if its an audio call later when it is supported
Copy link

Choose a reason for hiding this comment

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

If you write re-add, it won't look like a typo of read.

src/tox.c Outdated
free(data);

if (error) {
LOG_ERR("Toxcore", "Error sending groupchat message... %u" , error);
LOG_ERR("Toxcore", "Error sending message to groupchat %u. Erorr number: %u", param1, error);
Copy link

Choose a reason for hiding this comment

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

Typo: s/Erorr/Error/

Comment on lines 175 to 172
LOG_ERR("Tox Callbacks", "Could not join group with type: %u", type);
LOG_ERR("Tox Callbacks", "Could not join group with type");
Copy link

Choose a reason for hiding this comment

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

Did you not mean to delete the " with type", too?

@ghost
Copy link

ghost commented Apr 26, 2020

Thanks for starting this!

Try to keep your log clean from the start.
And please stay within 80 char line length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant