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

refactor: put the default host in a variable and then reuse it #197

Merged

Conversation

AbdulrhmanGoni
Copy link
Contributor

The default host http://127.0.0.1:11434 is hard-coded in multiple places,
So, Put it in one variable in a central place (src/defaultHost.ts file) and then reuse it in those places.

@Mustafiz04
Copy link

@AbdulrhmanGoni tests are failing because of package version mismatch.
Rebase with master and run npm i

Comment on lines 1 to 2
export const defaulPort = '11434';
export const defaultHost = `http://127.0.0.1:${defaulPort}`;

Choose a reason for hiding this comment

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

better you can give generic name constant.js that store all the constant here of defaultHost.js

There is wrong spelling too.

Suggested change
export const defaulPort = '11434';
export const defaultHost = `http://127.0.0.1:${defaulPort}`;
export const port = '11434';
export const host = `http://127.0.0.1:${port}`;

Copy link
Contributor Author

@AbdulrhmanGoni AbdulrhmanGoni Jan 28, 2025

Choose a reason for hiding this comment

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

better you can give generic name constant.js that store all the constant here of defaultHost.js

OK, I renamed defaultHost.ts file to constant.ts

And i fixed the wrong spelling, But i think there is no need to rename the variables defaultHost
and defaultPort to host and port because there are already variables with these names
in src/utils.ts file (one of the places where defaultHost and defaultPort are used in)

The default host 'http://127.0.0.1:11434' is hardcoded in
multiple places, So, Put it in one variable in a central place
(`src/defaultHost.ts` file) and reuse it in those places.
Rename `src/defaultHost.ts` to `src/constant.ts`
and fix the wrong spelling `defaulPort` typo by
just ranaming it to `defaultPort`.
@AbdulrhmanGoni AbdulrhmanGoni force-pushed the refactor/repeated-value-into-one-variable branch from 2c52e68 to 3dcbd0b Compare January 28, 2025 11:45
@AbdulrhmanGoni
Copy link
Contributor Author

AbdulrhmanGoni commented Jan 28, 2025

@AbdulrhmanGoni tests are failing because of package version mismatch. Rebase with master and run npm i

I understood package version mismatch causes the tests to fail,
I rebased with master (main branch i think) as you said run npm i, But tests still fail !!

Copy link
Collaborator

@BruceMacD BruceMacD left a comment

Choose a reason for hiding this comment

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

Thanks for fixing the build errors too.

@BruceMacD
Copy link
Collaborator

Merging this one for now, fixing the build errors in a fast follow.

@BruceMacD BruceMacD merged commit 9ff1621 into ollama:main Jan 28, 2025
1 of 3 checks passed
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.

3 participants