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

Consider merge up to https://github.com/rust-unofficial/patterns #7

Open
tesuji opened this issue May 12, 2020 · 8 comments
Open

Consider merge up to https://github.com/rust-unofficial/patterns #7

tesuji opened this issue May 12, 2020 · 8 comments

Comments

@tesuji
Copy link

tesuji commented May 12, 2020

No description provided.

@simonsan
Copy link

Hey, I'm one of the maintainers of said repository and I think it would make sense to "merge" this repository as we are working on bringing rust-unofficial to rust-lang. What's your opinion on this @lpxxn ? Umbrella issue is here: rust-unofficial/patterns#116

@lpxxn
Copy link
Owner

lpxxn commented Feb 22, 2021

it's ok. but how to do this, copy all code to rust-unoffical/patterns ?

@simonsan
Copy link

it's ok. but how to do this, copy all code to rust-unoffical/patterns ?

Hmm, as we have CI checks in place and we (maintainers) and others want to read over what is being merged into the repository it might be best, if we (I can help you there) open PRs for each pattern. I know it sounds like a lot of work, but we'll manage and it will be for the good. :)

Also another question is the differences in licensing. Patterns is licensed under MPL-2.0 whereas contents of this repository are licensed under GPLv3. It might make sense to state in each PR then, that you relicense the PRed content under MPL-2.0 for the patterns repository.

If you agree I can start creating PRs for each of the patterns. :)

@simonsan
Copy link

I've reorganized the structure of the patterns repository so we can easier PR these patterns into their corresponding categories in rust-unofficial/patterns#219

@simonsan
Copy link

We should also be aware, that some Patterns might need changes, as we follow our template style. But filling that out we can do collaboratively with the user base. Might be better, if I PR them though. 🤔 If you are willing and stating to relicense the content in the corresponding PRs. So it's overall less work for you :)

@pickfire
Copy link

pickfire commented Feb 22, 2021

@lpxxn Some of the patterns here felt like directly taken from another language which does not seeemed fit so well in rust which is probably the reason why it wasn't included in the rust patterns at first. One reason could be ergonomic. We should think twice before including all of them, maybe some might be good like State (although the example isn't good), but some others looked a bit weird (lots of dyn), then we are on the road to do more reflection magic and remove the compile time safety rust provides.

Imagine having lifetime, borrowing, ownership pattern in java, what would happened?

@simonsan Please try to make sure we have a good example or even better, it is a pattern being used in a popular crate, then I would say it is good to merge it, otherwise it feels weird to have a hard direct port.

@lpxxn
Copy link
Owner

lpxxn commented Feb 23, 2021

@pickfire i agree with you,this repositories was written when i was learning rust,and some codes are reall bad.
@simonsan i relicense to MPL-2.0,you can use and change my code.

@simonsan
Copy link

simonsan commented Feb 23, 2021

@lpxxn Thank you :)

The point is, sometimes people just miss examples to write the corresponding articles, I did it now in the way, that I opened an issue for every missing article that we find examples in here and used them. My hope is, that more people write some articles about these patterns in the future because of the reason that they easily found an example for it, that they could still improve, if they wanted to. So @lpxxn @pickfire don't worry, we won't merge it into the repository immediately. But as examples to discuss about and improve upon together they are really nice.

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

No branches or pull requests

4 participants