Skip to content

Conversation

@leandronsp
Copy link
Contributor

This PR adds an implementation in Go for Deque (double-ended queue).

It covers some test scenarios using the library testify which makes easier to write unit tests in Go.

@leandronsp leandronsp changed the title Add golang implementation for Deque Add Deque in Go Sep 28, 2023
@leandronsp leandronsp changed the title Add Deque in Go Add deque in Go Sep 28, 2023
Copy link
Owner

@kelvins kelvins left a comment

Choose a reason for hiding this comment

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

Hi @leandronsp, could you please add package main and apply gofmt

@leandronsp
Copy link
Contributor Author

My bad, I forgot to apply go fmt. Gonna do that and once it passed, I'll do for the other PR's I left open.

@leandronsp
Copy link
Contributor Author

@kelvins it seems that the testify package is making the lint phase fail. I'll remove the testify and rewrite the tests following other go files in this project, I'll work on that in the next couple of days

@leandronsp
Copy link
Contributor Author

Otherwise, do you think it's a good idea adding testify assert to the pipeline? I think it helps writing good tests in Go. But I'm okay in case you think we should not use it for now

@kelvins
Copy link
Owner

kelvins commented Oct 2, 2023

I agree that testify is a great library for performing assertions, but since this repository is intended for study purposes, I suggest avoiding any kind of external library. Using external libraries would make it difficult for newcomers to contribute, it would be hard to run the code online (like in playgrounds) and harder to maintain (to keep library versions updated).

I would suggest implementing tests using the standard library or implementing a main function and printing some examples (not ideal but it is easy to check).

@leandronsp
Copy link
Contributor Author

okay @kelvins , I'll apply the adjustments and update the PR, thanks

@leandronsp
Copy link
Contributor Author

@kelvins kelvins merged commit 474b89f into kelvins:main Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants