Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(ontology-find-icon-helper): add package #117

Merged
merged 12 commits into from
Dec 2, 2024

Conversation

ACoolmanTelicent
Copy link
Contributor

@ACoolmanTelicent ACoolmanTelicent commented Nov 28, 2024

Useful: Diff of changes MINUS initial package config files

Ticket: https://telicent.atlassian.net/browse/TELFE-772

Goal

  1. Created new package ontology-find-icon-helper that contains....
  2. ...these helper fns for FE apps:
    1. init() — fetching/storing style/icons data from OntologyService
    2. findByClassUri() — accessing fetched/stored style/icons by classURI

Testing Method

  • updated Design-system and Search app - and ensured icons/styles are working
  • Test coverage:
    • 100% Statements 62/62
    • 87.5% Branches 14/16
    • 100% Functions 13/13
    • 100% Lines 51/51

Engineering notes

Adding to app

How icons are loaded via direct use of ontologyFindIconHelper:

How icons are loaded via re-usable component:

Out-of-scope

  1. Publishing; I'll wait until Design-system and Search app PRs are approved
  2. Design system refactor — new PR will do some light tweaks
  3. Search app — new PR will use this code

Related PRs:

  1. https://github.com/Telicent-io/telicent-search/pull/690
  2. fix(package.json): [TELFE-563] test rdf-libraries telicent-ds#166

🕵️ Review Guidance

General guidance

  • Generally, approve a PR if it makes the system better, even if it's not perfect. — Google: The Standard of Code Review
  • The aim of both PR AUTHOR and PR REVIEWER is to get the code merged
  • Aim for consensus, defined as everyone can live with the outcome

For PR REVIEWER:

  1. Read the ticket & description
  2. Review the code
  3. Request any changes that are essential.
  4. For non-essential comments:
    • Use prefixes, e.g. "nit: change to Pascal case"
      • nit: small, non-essential change
      • obs: just an observation, doesn't affect the PR
      • idea: a suggestion to think about
      • q: questions
    • Use modifiers, e.g. "obs[pr-owner-resolve]: Jim is also editing this file"
      • pr-author-resolve PR author can resolve after reading
      • pr-author-delete (rare) delete after reading to avoid confusion
  5. try to add at least a helpful comment per ~500 lines; less if the PR is already busy

For PR AUTHOR:

  1. Aim for enough detail in PR description for things to go smoothly
  2. After requested changes, re-request a review (so the PR shows up in reviews-requested:@me)

@ACoolmanTelicent ACoolmanTelicent self-assigned this Nov 28, 2024
@ACoolmanTelicent ACoolmanTelicent marked this pull request as draft November 29, 2024 11:14
@ACoolmanTelicent
Copy link
Contributor Author

Converted to draft - as I've just realised the code (useOntologyStyles) that this helper deprecates - actually has more opinion that needs to be ported over.

Plus, I might need to do some small changes for reactivity (which we were originally trying to avoid)

ACoolmanTelicent added 4 commits November 29, 2024 16:43
- Previously used by useOntologyStyles
- Copied into ontology-find-icon-helper to achieve parity
- Very likely this logic will be changed or removed at a later date
@ACoolmanTelicent ACoolmanTelicent merged commit a43f37b into main Dec 2, 2024
3 checks passed
@ACoolmanTelicent ACoolmanTelicent deleted the TELFE-654/ontology-find-icon-helper branch December 2, 2024 13:05
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants