ecg-icas / icas

iCAS documentation and changelogs
4 stars 2 forks source link

Dr/oauth2 improvements #63

Closed remenska closed 2 years ago

remenska commented 2 years ago
twessling-icas commented 2 years ago

The added image names our system 'iCAS' - I don't think our clients know us by this name? Should we make that Admarkt or whutnot? Furthermore, I think adam has some nice minor suggestions still. Otherwise looks quite good imo

remenska commented 2 years ago

The added image names our system 'iCAS' - I don't think our clients know us by this name? Should we make that Admarkt or whutnot? Furthermore, I think adam has some nice minor suggestions still. Otherwise looks quite good imo

not sure about this really: is our product Admarkt even outside of BNL? happy to change it, domains are admarkt so it makes sense

twessling-icas commented 2 years ago

The added image names our system 'iCAS' - I don't think our clients know us by this name? Should we make that Admarkt or whutnot? Furthermore, I think adam has some nice minor suggestions still. Otherwise looks quite good imo

not sure about this really: is our product Admarkt even outside of BNL? happy to change it, domains are admarkt so it makes sense

Good point, I'm honestly not sure what the best would be - and this repository is still ecg-icas/icas... For now we can probably keep it then, and let's put this forward as well to talk about our (tech) representation outward; we're going to onboard more german customers and other tenants will probably follow in the coming years... Best to have a single name for our platform. If that will be iCAS, all good, but let's make it a conscious decision and be consistent with it in our docs.

twessling-icas commented 2 years ago

scrap the scrapping, use scraping instead :trollface:

remenska commented 2 years ago

scrapping

not 100% sure what you mean :D but I'm not scraping anything related to campaign APIs right now :D

twessling-icas commented 2 years ago

you've got some sentences like It should not be used for scrapping the entire inventory of ads from that campaign. I believe this should be scraping and not scrapping

remenska commented 2 years ago

you've got some sentences like It should not be used for scrapping the entire inventory of ads from that campaign. I believe this should be scraping and not scrapping

yes, it's on multiple places with those advanced views APIs (that will be revisited) and are out of scope of the goal of this PR

twessling-icas commented 2 years ago

the diagram still mentions POST /auth/cookie, I'd rather see this less transparent... like or smth like that. Next to that, the diagram uses channable.com as an example third-party; do we want to keep this or should we introduce our own at "your-company.com/code" etc?

twessling-icas commented 2 years ago

you've got some sentences like It should not be used for scrapping the entire inventory of ads from that campaign. I believe this should be scraping and not scrapping

yes, it's on multiple places with those advanced views APIs (that will be revisited) and are out of scope of the goal of this PR

that is fine, but the typo remains.

remenska commented 2 years ago

you've got some sentences like It should not be used for scrapping the entire inventory of ads from that campaign. I believe this should be scraping and not scrapping

yes, it's on multiple places with those advanced views APIs (that will be revisited) and are out of scope of the goal of this PR

that is fine, but the typo remains.

yes, and I'm absolutely sure there are other typos in our documentation, but it is outside of scope (unless I missed the typo inside the oauth2 part). If you want, I can open a separate ticket (don't want to address those endpoints now for reasons I stated above)

twessling-icas commented 2 years ago

you've got some sentences like It should not be used for scrapping the entire inventory of ads from that campaign. I believe this should be scraping and not scrapping

yes, it's on multiple places with those advanced views APIs (that will be revisited) and are out of scope of the goal of this PR

that is fine, but the typo remains.

yes, and I'm absolutely sure there are other typos in our documentation, but it is outside of scope (unless I missed the typo inside the oauth2 part). If you want, I can open a separate ticket (don't want to address those endpoints now for reasons I stated above)

Ah, right, because with the later changes of this PR those sentences remain untouched. Fine, leave it for next time :)

remenska commented 2 years ago

you've got some sentences like It should not be used for scrapping the entire inventory of ads from that campaign. I believe this should be scraping and not scrapping

yes, it's on multiple places with those advanced views APIs (that will be revisited) and are out of scope of the goal of this PR

that is fine, but the typo remains.

yes, and I'm absolutely sure there are other typos in our documentation, but it is outside of scope (unless I missed the typo inside the oauth2 part). If you want, I can open a separate ticket (don't want to address those endpoints now for reasons I stated above)

Ah, right, because with the later changes of this PR those sentences remain untouched. Fine, leave it for next time :)

yes exactly, we're re-address those APIs anyways and I'll open a ticket for the typos :D I think what you stumbled on was autogenerated txt files or so

remenska commented 2 years ago

@twessling-icas can I get unblocked?

twessling-icas commented 2 years ago

@twessling-icas can I get unblocked?

I also had some more comments on the png image, it mentions channable.com a lot, and still shows the POST /auth/cookie

remenska commented 2 years ago

@twessling-icas can I get unblocked?

I also had some more comments on the png image, it mentions channable.com a lot, and still shows the POST /auth/cookie

POST /auth/cookie i wanna show because it;s useful, as we get more and more tenants play the intermediary role of API partners. It doesn't hurt a lot

remenska commented 2 years ago

@twessling-icas can I get unblocked?

I also had some more comments on the png image, it mentions channable.com a lot, and still shows the POST /auth/cookie

POST /auth/cookie i wanna show because it;s useful, as we get more and more tenants play the intermediary role of API partners. It doesn't hurt a lot

what's wrong with channable as example of a well known third party? But fine I'll make the final last change and I hope this PR doesnt linger and lose value further

remenska commented 2 years ago

done