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

may lychee third party link updates #3782

Merged
merged 3 commits into from
May 17, 2024
Merged

may lychee third party link updates #3782

merged 3 commits into from
May 17, 2024

Conversation

christinaausley
Copy link
Contributor

Description

May Lychee review for broken third-party links.

When should this change go live?

  • This is a bug fix, security concern, or something that needs urgent release support.
  • This is already available but undocumented and should be released within a week.
  • This on a specific schedule and the assignee will coordinate a release with the DevEx team. (apply hold label or convert to draft PR)
  • This is part of a scheduled alpha or minor. (apply alpha or minor label)
  • There is no urgency with this change and can be released at any time.

PR Checklist

  • My changes are for an already released minor and are in /versioned_docs directory.
  • My changes are for the next minor and are in /docs directory (aka /next/).

@christinaausley christinaausley added kind/bug Issues related with bugs in the documentation component:docs Documentation improvements, including new or updated content labels May 8, 2024
@christinaausley christinaausley self-assigned this May 8, 2024
@christinaausley
Copy link
Contributor Author

christinaausley commented May 8, 2024

@chillleader @RomanJRW looping you in as there are a few corrections for Connectors and Optimize, but let me know if you'd like another team member to take a look 👍

@christinaausley christinaausley requested review from RomanJRW, YanaSegal and chillleader and removed request for YanaSegal May 8, 2024 17:53
@christinaausley
Copy link
Contributor Author

CC @leiicamundi -- thanks again for opening the notes on Lychee. As a reminder, I have an hour on my calendar set aside each month to clean these up.

@akeller
Copy link
Member

akeller commented May 8, 2024

Most of my questions about this PR stem from needing to fully understand how we can pick specific versions of 3rd party tech to reference in our docs. Theoretically, we can't pick "current" either unless we know the dependencies are kept up to date across all our supported versions.

@christinaausley can you provide more context about how you chose the links with numbered versions in them? Are you matching it to the supported environments or dependencies pages?

Copy link
Contributor

@RomanJRW RomanJRW left a comment

Choose a reason for hiding this comment

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

Couple of small comments from me. I left them only once but they obvs apply to all the versions of the files

@@ -52,4 +52,4 @@ Apart from caching issues, there is the following list of known data update limi

The configuration of Optimize allows you to define when the history cleanup is triggered using cron expression notation. However, the values are incorrectly interpreted in Optimize. For example, the `historyCleanup.cronTrigger` configuration has the default value `0 1 * * *`, which should be 01:00 AM every day. Unfortunately, a bug causes this to be interpreted as every hour.

To fix this, use the Spring [cron expression notation](https://docs.spring.io/spring/docs/current/javadoc-api/org/springframework/scheduling/support/CronSequenceGenerator.html). For instance, the default value for `historyCleanup.cronTrigger` would then be `0 0 1 * * *`.
To fix this, use the Spring [cron expression notation](https://docs.spring.io/spring-framework/docs/5.1.0.RELEASE/javadoc-api/org/springframework/scheduling/support/CronSequenceGenerator.html). For instance, the default value for `historyCleanup.cronTrigger` would then be `0 0 1 * * *`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not going back to check which version of Spring we used on version 2.3... 😆

I would consider removing it here and in all the files below though, I don't see a big value including a link here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are 30 references of this across the docs -- would you prefer to remove the entire ### Misinterpreted cron expressions section? If so, do you feel users really need this context often, or it is rarely used?

If not, would you prefer to link to something like https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/scheduling/support/CronExpression.html?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a good judgement on how often this is used, but I am sure it was probably added in the first place because it would have confused users at some point in time. I would be happy to change it to the docs link you suggested. This is the correct cron expression type and is not versioned docs. Thanks for the idea!

Copy link
Member

@akeller akeller left a comment

Choose a reason for hiding this comment

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

I didn't do an exhaustive check of each individual URL, but I do appreciate that this PR also removes links and reduces what we need to maintain! Thank you!

@christinaausley christinaausley merged commit 38a7f4b into main May 17, 2024
8 checks passed
@christinaausley christinaausley deleted the may-lychee-review branch May 17, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:docs Documentation improvements, including new or updated content kind/bug Issues related with bugs in the documentation
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants