Linguist - Added new dedicated module for the Linguist Tags Processor #390
Conversation
Unnecessary ChangesetsThe following package(s) are private and do not need a changeset:
Changed Packages
|
|
Hi @bforbis, going to need your help with reviewing this, please? No rush, as you have time. |
awanlin
force-pushed
the
topic/add-linguit-processor-module
branch
from
e4274ee
to
c7d6eb4
Compare
awanlin
force-pushed
the
topic/add-linguit-processor-module
branch
from
c7d6eb4
to
ab5b86f
Compare
Signed-off-by: Andre Wanlin <[email protected]> Ran prettier and dedupe Signed-off-by: Andre Wanlin <[email protected]> Added missing packages Signed-off-by: Andre Wanlin <[email protected]> Added tag processor to example app Signed-off-by: Andre Wanlin <[email protected]> Fixed service to service auth error Signed-off-by: Andre Wanlin <[email protected]> Ran dedupe Signed-off-by: Andre Wanlin <[email protected]> Fixed type errors Signed-off-by: Andre Wanlin <[email protected]> Ran prettier Signed-off-by: Andre Wanlin <[email protected]> Updated API report Signed-off-by: Andre Wanlin <[email protected]>
awanlin
force-pushed
the
topic/add-linguit-processor-module
branch
from
bd688db
to
17d68f8
Compare
|
@awanlin left some comments for you. Apologize for the delay in review, the weather has been really nice outside :) |
No worries but I don't think you hit submit as I don't see the comments? 🤔 |
|
|
||
| Then you will need to make the following changes in your `/packages/backend/src/plugins/catalog.ts` file: | ||
|
|
||
| ```ts |
There was a problem hiding this comment.
I don't think this example is needed anymore, you have another one right below which shows the same setup in diff view
|
|
||
| The `shouldProcessEntity` is a function you can pass into the processor which determines which entities should have language tags fetched from linguist and added to the entity. By default, this will only run on entities of `kind: Component`, however this function let's you fully customize which entities should be processed. | ||
|
|
||
| > Note: this is not currently supported with the new backend system |
There was a problem hiding this comment.
Does this need to change with the new backend system? I'm not very familiar with it, but is there a way to pass in a function when doing backend.add()?
There was a problem hiding this comment.
Should we just remove the feature instead? When I wrote this originally, I was trying to be overly helpful, but I'm actually not using it myself.
| * limitations under the License. | ||
| */ | ||
|
|
||
| import { TaskScheduleDefinition } from '@backstage/backend-tasks'; |
|
|
||
| import { TaskScheduleDefinition } from '@backstage/backend-tasks'; | ||
| import { HumanDuration } from '@backstage/types'; | ||
| import { Options as LinguistJsOptions } from 'linguist-js/dist/types'; |
| "@backstage/backend-test-utils": "^0.3.7", | ||
| "@backstage/cli": "^0.26.3", | ||
| "js-yaml": "^4.1.0", | ||
| "linguist-js": "^2.5.3" |
There was a problem hiding this comment.
What strategy is there to keep this dependency in sync with the one in linguist-backend? Part of the problem with splitting these out into many different modules is it opens up opportunity for dependency drift.
| @@ -88,7 +88,10 @@ export interface LinguistTagsProcessorOptions { | |||
| * add the languages to the entity as searchable tags. | |||
| * | |||
| * @public | |||
| * */ | |||
| * | |||
| * @deprecated Use `@backstage-community/plugin-catalog-backend-module-linguist-tags-processor` instead, | |||
There was a problem hiding this comment.
How long should this stay deprecated before removing? I assume that would need a major version bump to this backend module, as that would be considered a breaking change.
| cacheTTL: { minutes: 0 } | ||
| ``` | ||
|
|
||
| #### `bytesThreshold` |
There was a problem hiding this comment.
One additional thing I was thinking could be useful here would be another version of this that is a percentageThreshold, which might be a better option for deciding when to include a language tag.
Possibly that could be it's own PR, but re-looking at this it just came up as an idea.
| const linguistData = await fetch(linguistApi, { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Authorization: `Bearer ${token}`, |
There was a problem hiding this comment.
I am now finally getting to the point of upgrading my backstage instance to the new backend, and this line seems to be the key to making this work with Auth, so thanks for this <3
Hey, I just made a Pull Request!
Added new dedicated module for the Linguist Tags Processor and deprecated the version in the Linguist Backend
✔️ Checklist
Signed-off-byline in the message. (more info)