Skip to content

Fix Singapore 2026 public holiday dates#311

Open
StephenMF-beep wants to merge 2 commits into
holidays:masterfrom
TandaHQ:SG-PH-update
Open

Fix Singapore 2026 public holiday dates#311
StephenMF-beep wants to merge 2 commits into
holidays:masterfrom
TandaHQ:SG-PH-update

Conversation

@StephenMF-beep
Copy link
Copy Markdown

Summary

  • Fix Deepavali 2026 date from Oct 8 to Nov 8
  • Fix Hari Raya Puasa 2026 date from Mar 20 to Mar 21

Test plan

  • Ran bundle exec ruby lib/validation/run.rb — Success
  • Ran bundle exec rspec — 50 examples, 0 failures

🤖 Generated with Claude Code

@StephenMF-beep
Copy link
Copy Markdown
Author

Changed the Singapore file to have the correct dates for Deepavali 2026 date from Oct 8 to Nov 8
Fix Hari Raya Puasa 2026 date from Mar 20 to Mar 21.

@ppeble
Copy link
Copy Markdown
Member

ppeble commented Mar 29, 2026

Hello! Can you please rebase? There are a lot of unrelated changes. Then I'll review and merge.

@StephenMF-beep
Copy link
Copy Markdown
Author

Hello! Can you please rebase? There are a lot of unrelated changes. Then I'll review and merge.

thanks for letting me know. I have rebased and pushed this branch back in to github.

@DagonWat
Copy link
Copy Markdown
Contributor

@StephenMF-beep I think something went wrong after rebase, please make sure you have only your changes

- Fix Deepavali 2026 date from Oct 8 to Nov 8
- Fix Hari Raya Puasa 2026 date from Mar 20 to Mar 21
- Add methods for Chinese New Year, Deepavali, Hari Raya Puasa/Haji, Vesak Day

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ppeble
Copy link
Copy Markdown
Member

ppeble commented Apr 1, 2026

Thank you for rebasing! I added a new job that tests against the downstream ruby gem and it seems to have caught something. Let me take a closer look and we can find out what needs to be updated.

@ppeble
Copy link
Copy Markdown
Member

ppeble commented Apr 1, 2026

Oh! It says it in the error, it is just buffered weird. I am going to try to address how it is shown but there is a problem with the format of the methods you added:

rake aborted!
ArgumentError: ArgumentError (ArgumentError)

          raise ArgumentError unless methods.all? do |name, pieces|
                ^^^^^^^^^^^^^
/home/runner/work/definitions/definitions/holidays/lib/holidays/definition/parser/custom_method.rb:34:in 'Holidays::Definition::Parser::CustomMethod#validate!'
/home/runner/work/definitions/definitions/holidays/lib/holidays/definition/parser/custom_method.rb:14:in 'Holidays::Definition::Parser::CustomMethod#call'
/home/runner/work/definitions/definitions/holidays/lib/holidays/definition/context/generator.rb:36:in 'block in Holidays::Definition::Context::Generator#parse_definition_files'
/home/runner/work/definitions/definitions/holidays/lib/holidays/definition/context/generator.rb:33:in 'Array#each'
/home/runner/work/definitions/definitions/holidays/lib/holidays/definition/context/generator.rb:33:in 'Holidays::Definition::Context::Generator#parse_definition_files'
/home/runner/work/definitions/definitions/holidays/Rakefile:66:in 'block (3 levels) in <top (required)>'
/home/runner/work/definitions/definitions/holidays/Rakefile:62:in 'Hash#each'
/home/runner/work/definitions/definitions/holidays/Rakefile:62:in 'block (2 levels) in <top (required)>'
/home/runner/work/definitions/definitions/holidays/vendor/bundle/ruby/4.0.0/gems/rake-13.3.1/exe/rake:27:in '<top (required)>'
/opt/hostedtoolcache/Ruby/4.0.2/x64/bin/bundle:25:in '<main>'
Tasks: TOP => generate => generate:definitions
(See full trace by running task with --trace)

The key of 'source' should instead be 'ruby'. I had this wild idea that we would potentially have other languages that could do their own functions and so we want to be specific, even though we only do 'ruby' right now.

Replace the instances of 'source' with 'ruby' and then push again and let's see what happens.

ttwo32
ttwo32 previously approved these changes Apr 4, 2026
@ppeble
Copy link
Copy Markdown
Member

ppeble commented Apr 27, 2026

@StephenMF-beep Hello! Just following up, the failure that we saw was because of the key, detailed here: #311 (comment)

If you update this and everything passes I would be happy to merge!

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@ppeble ppeble left a comment

Choose a reason for hiding this comment

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

There seems to be a discrepancy between the month definition name between 'Chinese New Year' and 'Lunar New Year', which is causing the tests to fail.

Comment thread sg.yaml
regions: ["sg"]
options: ["observed"]
expect:
name: "Lunar New Year's Day"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This name does not match the name in the change above, which is Chinese New Year. Same for the 'second day' version. The names do not match between the definitions and the tests.

Comment thread sg.yaml
regions: ["sg"]
options: ["observed"]
expect:
name: "The second day of Lunar New Year"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same issue as above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants