os-js / osjs-client

OS.js Client Module
https://manual.os-js.org/
Other
31 stars 31 forks source link

feat(search):add feature search adapter support #205

Closed mahdi00021 closed 1 year ago

mahdi00021 commented 1 year ago

please check PR


Ref: #204 #202 #201 #199

mahdi00021 commented 1 year ago

@andersevenrud thanks for merge

andersevenrud commented 1 year ago

Thanks for the contribution.

There was some styling issues in the test file, but I just followed this up myself: https://github.com/os-js/osjs-client/commit/decda8d44a023359e1759c42be7523dc6df5f6d3

mahdi00021 commented 1 year ago

Thanks for the contribution.

There was some styling issues in the test file, but I just followed this up myself: decda8d

Because ESLint fixes the src folder, I did not apply it to the test file

andersevenrud commented 1 year ago

Because ESLint fixes the src folder, I did not apply it to the test file

And I've since fixed this.

However, since it didn't test the tests folder it does not mean it does not apply there. Use your best judgement (I've also commented quite a few times about this, i.e: about correct indentation and such -- which you can do with your eyes. No need for eslint) :)

mahdi00021 commented 1 year ago

Because ESLint fixes the src folder, I did not apply it to the test file

And I've since fixed this.

However, since it didn't test the tests folder it does not mean it does not apply there. Use your best judgement (I've also commented quite a few times about this, i.e: about correct indentation and such -- which you can do with your eyes. No need for eslint) :)

.

Because ESLint fixes the src folder, I did not apply it to the test file

And I've since fixed this.

However, since it didn't test the tests folder it does not mean it does not apply there. Use your best judgement (I've also commented quite a few times about this, i.e: about correct indentation and such -- which you can do with your eyes. No need for eslint) :)

I set the indentation manually once, for other files your friend complained why the file changed :))

andersevenrud commented 1 year ago

I set the indentation manually once, for other files your friend complained why the file changed

It was not a complaint, just a mere suggestion for improvement or inquiry about a change. And it was not even related to this.

But it doesn't really matter now that it's been followed up manually, and I've enforced all style checks to run in the tests directory in any scenario.

mahdi00021 commented 1 year ago

I set the indentation manually once, for other files your friend complained why the file changed

It was not a complaint, just a mere suggestion for improvement or inquiry about a change. And it was not even related to this.

But it doesn't really matter now that it's been followed up manually, and I've enforced all style checks to run in the tests directory in any scenario.

No, I didn't say now. I before edited Indentations some files manually and push them. Your friend said why did you change the rest of the files?

I do not intend to argue and I did not say that there is hostility I mean Because you said that the indentation test file is not correct I said that if I want to modify it, your friend will probably tell you why the file was changed and I should have started from the

beginning, that's why I didn't edit the style files except for the src folder.

andersevenrud commented 1 year ago

It was simply this:

  1. You removed package-lock.json, which was an error by you
  2. He simply commented that there was an extra new line

@ajmeese7 is an official maintainer of this project. He just followed up with some reviews and general friendly and insightful comments on how to improve your contribution. There was no hostility or complaints.

In fact, none of the discussions had have been complaints. Just a part of a review process where the goal is to have a friendly talk about how to get the best result possible.

I'm locking this PR now because this is now an off-topic discussion.