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

Discussions #9

Open
drewburlingame opened this issue Dec 10, 2022 · 3 comments
Open

Discussions #9

drewburlingame opened this issue Dec 10, 2022 · 3 comments

Comments

@drewburlingame
Copy link

Hi @VaclavElias, we're evaluating this library to use for our integrations. What's the best way to ask questions? Should we create issues or would you prefer to enable discussions in the repo or would you prefer something like gitter or discord?

Some of the questions:

  • There are comments in the code about that methods should be tested. Do you know how you plan to implement and execute the tests?
  • There are open issues to refactor components that we would be using. Will they be major breaking changes? Do you have an idea of the timeframe in which you plan to implement them?
  • Are you open to PRs to better support NRT in the APIs? I noticed a number of warning about possible null refs.

Cheers,
Drew

@VaclavElias
Copy link
Member

Hi @drewburlingame,

Let's start with Discussions and see if that works. I enabled it in this repository.

I have been working with multiple APIs and I have been trying to find some common pattern, which could be used architecture wise or reusability wise across multiple (independent) API repositories.

I moved and started refactoring this Bullhorn repo in the new place https://github.com/api-bureau which is getting aligned to the other repositories architecture wise. The new Bullhorn repository is currently private, let me review it and I will make it also MIT public.

My goal is to find some good and stable architecture but it is still work in progress. I use in my projects confluence-api, bullhorn-api (temporarily private), e-days-api, cloudcall-api (all here https://github.com/api-bureau). They are stable in the way we are using them internally but if you would like to use bullhorn-api I would recommend to fork it (it is going to be MIT) and do whatever you want to or take any part of it. In case you would like to contribute/improve this new version then I am happy to collaborate 🙂.

Regarding your questions:

There are comments in the code about that methods should be tested. Do you know how you plan to implement and execute the tests?

I think we can't test too much here, I would like some better error handling, so the client can easily identify any possible issue (Bullhorn, parsing, ..) and that needs to be tested, Unit tests should be sufficient, wherever they are needed. If we talk specifically about this below, that is used in my other project I call Bullhorn Query (private). That is just UI like simplified Postman, where you can write any Bullhorn API query and see the output in the tabular way, that's why I need a dynamic query which is then flattened. This app is easier to use by anyone, test Bullhorn queries quickly or check Bullhorn data. I would like to make this tool also MIT and refactor it to MAUI. This below isn't used anywhere else.

//ToDo Test this
public async Task<DynamicQueryResponse> ApiCallToDynamicAsync(string query, int count, int start = 0)

There are open issues to refactor components that we would be using. Will they be major breaking changes? Do you have an idea of the timeframe in which you plan to implement them?

Yes, there will be. The reason is that I don't like the current architecture I made but I won't be doing it in this repository. The new version will be available here https://github.com/api-bureau/bullhorn-api once I make it public. This is still a work in progress and I don't have a timeframe yet. Any major breaking changes I made were very easy to fix in our projects which are using these libraries. I will try to make this public this week.

Are you open to PRs to better support NRT in the APIs? I noticed a number of warning about possible null refs.

Yes, I am happy to collaborate, but not this repo as I am not going to maintain it as I mentioned above. The new version is going to be here https://github.com/api-bureau/bullhorn-api. I would like to get rid of all warnings and properly handle any NRT. PRs are welcome.

@drewburlingame
Copy link
Author

HI @VaclavElias,

Thanks for the detailed response. It's very helpful to know that I shouldn't spend much time on this repo. I'll use it as a guide as I spike out an integration with Bullhorn.

Re: tests, can I suggest using a tool like WireMock.Net for testing? I had some ideas to contribute to this repo (which I obviously won't now) but the risk of modification without adequate testing made it feel too risky.

Definitely looking forward to seeing what you have in the new repo when it's available.

@VaclavElias
Copy link
Member

Hi @drewburlingame,

This is now public https://github.com/api-bureau/bullhorn-api

I consider it as better version but there is still lots of work ahead, including architecture improvements and resolve these from NDepend report.

image

Thank you for mentioning https://github.com/WireMock-Net/WireMock.Net, I will use it.

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

2 participants