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

Appears to be copy pasta in Docs.Tutorials - Tutorial4 #6342

Closed
F0b0s opened this issue Jan 10, 2023 · 7 comments · Fixed by #6343
Closed

Appears to be copy pasta in Docs.Tutorials - Tutorial4 #6342

F0b0s opened this issue Jan 10, 2023 · 7 comments · Fixed by #6343

Comments

@F0b0s
Copy link
Contributor

F0b0s commented Jan 10, 2023

When i was looking #4116 i found that the same tests set probably was copy pasted for tutorial 4 and is not used by documentation.

Tutorial only use this references:
Tutorial4/DeviceGroupSpec.cs?name=group-query-integration-test
But Tutorial4/DeviceGroupSpec.cs has a set of tests not included in group-query-integration-test region and these tests are the same as Tutorial3/DeviceGroupSpec but without fixes from #4116.

I think it would be better to delete these tests.

F0b0s added a commit to F0b0s/akka.net that referenced this issue Jan 10, 2023
@F0b0s F0b0s mentioned this issue Jan 10, 2023
5 tasks
@F0b0s
Copy link
Contributor Author

F0b0s commented Jan 16, 2023

@Aaronontheweb can you take a look, please?

@Aaronontheweb
Copy link
Member

Why would we remove those tests if they're doing something functional? @F0b0s

@Aaronontheweb
Copy link
Member

What's the real issue here, in other words?

@F0b0s
Copy link
Contributor Author

F0b0s commented Jan 16, 2023

Some tests from
Tutorial4/DeviceGroupSpec.cs
are identical to
Tutorial3/DeviceGroupSpec

I suppose that someone, who created the Tutorial4, took Tutorial3 as the base, added some tests but forgot to remove copied tests. And this tests are not used in documentation. So i decided to remove them. Final decision up to you.

@F0b0s
Copy link
Contributor Author

F0b0s commented Jan 16, 2023

@Aaronontheweb
Copy link
Member

Aha - got it.

@Aaronontheweb
Copy link
Member

@F0b0s merged! Thanks for your contribution!

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 a pull request may close this issue.

2 participants