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

[ci] Only lint changelog in Node 16 #2147

Closed
wants to merge 1 commit into from

Conversation

sjchmiela
Copy link
Contributor

Why

lint-changelog tends to sometimes fail on Node 18 (see this). I don't want to debug why.

How

After noticing scripts/tsconfig.json mentions "node16", I figured maybe scripts are supposed to only run on Node 16. Adjusted matrix and job spec.

"module": "node16",
"moduleResolution": "node16",

Test Plan

I mean, it's linting changelog…

@sjchmiela sjchmiela added the no changelog PR that doesn't require a changelog entry label Dec 12, 2023
Copy link

⏩ The changelog entry check has been skipped since the "no changelog" label is present.

Copy link

Size Change: -1.78 kB (0%)

Total Size: 55.5 MB

Filename Size Change
./packages/eas-cli/dist/eas-linux-x64.tar.gz 55.5 MB -1.78 kB (0%)

compressed-size-action

Copy link

codecov bot commented Dec 12, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f0012f5) 54.20% compared to head (269ac29) 54.20%.

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #2147    +/-   ##
========================================
  Coverage   54.20%   54.20%            
========================================
  Files         509      509            
  Lines       18657    18657            
  Branches     3736     3927   +191     
========================================
  Hits        10112    10112            
+ Misses       8524     7856   -668     
- Partials       21      689   +668     

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

@sjchmiela sjchmiela marked this pull request as ready for review December 12, 2023 20:36
Copy link
Member

@szdziedzic szdziedzic left a comment

Choose a reason for hiding this comment

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

I think this is good for now, but in my opinion, we should try to make it work with Node 18 because Node 16 is no longer supported. I believe we should also add support for Node 20 run our test workflows on Node 16,18,20, and drop support for Node 14. WDYT?

Feel free to merge this as it is. We can figure it out later on.

@szdziedzic
Copy link
Member

Ok, I think that it broke in Node 18.19.0. I was able to replicate it locally.

@szdziedzic
Copy link
Member

WDYT about #2153?

@sjchmiela
Copy link
Contributor Author

Superseded by #2153

@sjchmiela sjchmiela closed this Dec 15, 2023
@sjchmiela sjchmiela deleted the stanley/lint-changelog-only-node-16 branch December 15, 2023 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog PR that doesn't require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants