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

(feat #70) Navbar configuration: #74

Merged

Conversation

AlDu2407
Copy link
Contributor

@AlDu2407 AlDu2407 commented Apr 1, 2024

  • Removed fixed links in Urchin.
  • Added configurable links property in Urchin configuration.
  • Updated application settings to include new links property.
  • Updated templ to add additional parameter for links array.
  • Updated app_settings_test.

(fix) Makefile:

  • Fixed version numbers in install-tools.

- Removed fixed links in Urchin.
- Added configurable `links` property in Urchin configuration.
- Updated application settings to include new links property.
- Updated `templ` to add additional parameter for links array.
- Updated `app_settings_test`.

(fix) Makefile:

- Fixed version numbers in `install-tools`.
@AlDu2407 AlDu2407 force-pushed the feat/70_configurable_links branch from 7dcd505 to bfdff1e Compare April 1, 2024 22:57
Copy link

codecov bot commented Apr 1, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 39.29%. Comparing base (86f0ef8) to head (bfdff1e).

❗ Current head bfdff1e differs from pull request most recent head f9eaf55. Consider uploading reports for the commit f9eaf55 to get more accurate results

Files Patch % Lines
app/image.go 0.00% 2 Missing ⚠️
app/contact.go 0.00% 1 Missing ⚠️
common/app_settings.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #74      +/-   ##
==========================================
- Coverage   43.42%   39.29%   -4.13%     
==========================================
  Files          16       15       -1     
  Lines         760      799      +39     
==========================================
- Hits          330      314      -16     
- Misses        386      441      +55     
  Partials       44       44              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@matheusgomes28 matheusgomes28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good Al, just need to increase the coverage now for tests.

I'm gonna start enforcing the coverage as we've had PRs from other people in the last couple of weeks that are forgetting to cover the newly added lines in tests, just so we at least have some sort of regression testing.

urchin_config.toml Show resolved Hide resolved
SourabHere and others added 5 commits April 9, 2024 21:44
I believe that `urchin_config.toml` is enough for a configuration file,
so we no longer need the env variable configs.
- Fixed minor bug in application start.
- Added suggested improvements from PR.
- Fixed tests for app settings.
Copy link
Owner

@matheusgomes28 matheusgomes28 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@matheusgomes28 matheusgomes28 merged commit 31a1c6c into matheusgomes28:main Apr 11, 2024
3 checks passed
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

Successfully merging this pull request may close these issues.

3 participants