Skip to content

Conversation

@jknedlik
Copy link
Contributor

@jknedlik jknedlik commented Oct 6, 2024

Hi! Back for more:

As you can see in the main, I let the function consume the a.room_dm_create and reset. It is then consumed by into_iter and moved back at the end, which will not really have an impact on cpu.

The Iterator extensions in this file is a lot of boilerplate and will lay here only until I find out how to do it even better.

This project logs extensively and I still have to find out what the best approach to more clearly show the algorithms is.
I've, done so by moving the text above the function, but still keeping it near(L#306-L309) . @8go Would that be fine for you?

There also is this conundrum:

Why have you not filtered/error'ed the invalid user_ids but let the process continue? I understand the logging, but why not bubble up Errors there?

Cheers 😁

8go and others added 3 commits October 6, 2024 17:43
- fixed bug in --log-level : now RUST_LOG is appropriately used as default
- fixed bug --verify, --verify emoji improved, --verify emoji-req works on Element Web app but still does not work on Element Android app
@jknedlik jknedlik marked this pull request as draft October 6, 2024 16:25
@jknedlik jknedlik marked this pull request as ready for review October 6, 2024 16:25
@jknedlik jknedlik marked this pull request as draft October 6, 2024 16:26
@8go
Copy link
Owner

8go commented Oct 21, 2024

There also is this conundrum:

Why have you not filtered/error'ed the invalid user_ids but let the process continue? I understand the logging, but why not bubble up Errors there?

I did that to be super extra safe. "It will fail, but let's try anyway". I think it is a valid strategy, to bubble up the Error immediately.

@8go
Copy link
Owner

8go commented Oct 21, 2024

Does this remove the names that consist only of whitespaces (Space, Tabs, ...)? It should.

@8go
Copy link
Owner

8go commented Oct 21, 2024

in src/main.rs you removed 2 lines L#3738-L3739

didn't you want to remove the 2 lines L#3735-L3736 instead ?

Please look at this.

@8go
Copy link
Owner

8go commented Oct 21, 2024

I like it 👏

Consider my comments above. Make any needed changes and submit PR. I am happy to merge !

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.

2 participants