Skip to content

Conversation

Enrico204
Copy link
Contributor

This pull request replaces %Y with %y in deadlines to fix the generated iCal, so that it can be imported in Google Calendar and other tools. Fixes #162 and supersedes #231

Future pull requests should pay attention to not have %Y (uppercasee Y) in the deadline, as only %y is allowed

@Enrico204
Copy link
Contributor Author

Enrico204 commented Jun 23, 2024

To be honest, @clementfung I think that we should fix the template itself to allow %Y, so that there will be no problem even if you merge deadlines with uppercase Y. Appending replace: '%Y', tyear after replace: '%y', tyear in all iCal templates should do the trick. However, I have no experience with those templates, and I have no way to test them (I think). Do you have some experience with those? Or, do you have a way to test that and revert the change if there is a problem?

(if we want to proceed and fix the template, we can close this pull request and open a new one)

@clementfung
Copy link
Collaborator

@Enrico204 Thank you for looking into this and sorry for the slow response!

To support rolling deadlines, we currently use '%Y' to designate the previous year, and '%y' to designate the current year: https://github.com/sec-deadlines/sec-deadlines.github.io/blob/master/static/js/main.js#L24

So accepting this change would be a breaking change for those deadlines. I also don't have much experience with generating and testing iCal, and I could take some time to look into that further (no promises on when that would be...).

So I think the options are:

  1. Remove support for '%Y' and find a better way to handle the years of rolling deadlines in the js file
  2. Remove support for rolling deadlines, and explicitly write the year for these deadlines moving forward.
  3. Find a way to fix the iCal template, as you suggested.

Option 2 is honestly not that bad, since we already have to update the "year" field every year anyways.
Any thoughts from you or others?

@Enrico204
Copy link
Contributor Author

Oh, I missed the difference between %Y and %y! Indeed, this PR cannot be merged.

I would suggest to fix the iCal template (option 3). The placeholder might be useful in some cases (like the one modified in this PR), and it doesn't seem to complex to support.

I suspect that defining a new variable and a new replacement should suffice. Something like {% assign tprevyear = 'now' | date: '%Y' | minus: 1 %} somewhere in the file (like in the same line as BEGIN:VCALENDAR), and then add replace: '%Y', tprevyear after replace: '%y', tyear. It will define a new variable using the current year and decrementing it by one, and then it will replace %Y with that year.

But I am quite busy right now, I can't try it :-(
Probably I can setup a test env after July 10th

pwilke added a commit to pwilke/pwilke.github.io that referenced this pull request Jul 4, 2024
@Enrico204
Copy link
Contributor Author

Closed as fixed by #324

@Enrico204 Enrico204 closed this Jul 16, 2024
@Enrico204 Enrico204 deleted the Enrico204-replace-Y-to-y branch July 16, 2024 21:04
pwilke added a commit to pwilke/pwilke.github.io that referenced this pull request Sep 15, 2025
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.

Can't import iCal into Google Calendar

2 participants