Skip to content
This repository has been archived by the owner on Jul 13, 2022. It is now read-only.

 Add unit tests #62

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

luisgrases
Copy link

No description provided.

@luisgrases luisgrases closed this Jun 24, 2022
@luisgrases luisgrases reopened this Jun 24, 2022
Copy link

@ThatOneCalculator ThatOneCalculator left a comment

Choose a reason for hiding this comment

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

Lgtm! :bowtie:

.gitignore Outdated
npm-debug.log*
yarn-debug.log*
yarn-error.log*
.idea

Choose a reason for hiding this comment

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

Suggested change
.idea
.idea

We should add documentation on our standards for formatting, but I gently suggest we terminate all files with a new line per unix standards

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the suggestion but I am going to leave it as it is. The deadline is approaching and we need this merged ASAP.

I left a TODO comment so we can handle this issue on a future PR :)

Choose a reason for hiding this comment

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

LGTM

Copy link

@Saklad5 Saklad5 left a comment

Choose a reason for hiding this comment

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

This is art.

@konstantinlindner
Copy link

LGTM

Copy link

@caelx caelx left a comment

Choose a reason for hiding this comment

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

Looks like the tests are currently failing, can you add some more tethers to the price.


test('it should be able to sell high', () => {
const finalPrice = getBitcoinPriceToday()
expect(finalPrice).toBeGreaterThan(initialPrice);
Copy link

@cbeninati cbeninati Jun 24, 2022

Choose a reason for hiding this comment

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

Suggested change
expect(finalPrice).toBeGreaterThan(initialPrice);
expect(finalPrice).toBeGreaterThan(initialPrice)

Consistent terminating characters.

yarn-error.log*
.idea

TODO: terminate files with new line per unix standard

Choose a reason for hiding this comment

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

The sensible choice of terms here would be end of line (EOL) given it is intended here to represent the end of a line, not the beginning of an always empty line.

Suggested change
TODO: terminate files with new line per unix standard
TODO: terminate files with end of line per unix standard

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

Successfully merging this pull request may close these issues.

8 participants