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

v5.6.2 #241

Merged
merged 1 commit into from
Apr 14, 2024
Merged

v5.6.2 #241

merged 1 commit into from
Apr 14, 2024

Conversation

mcbouslog
Copy link
Contributor

New patch version with:

  • Bump github/codeql-action from 2 to 3
  • Update README publishing instructions
  • Fix to ignore null items in API responses

@mcbouslog mcbouslog requested a review from a team April 8, 2024 16:02
@shaunanoordin shaunanoordin self-assigned this Apr 11, 2024
@shaunanoordin shaunanoordin self-requested a review April 11, 2024 15:12
Copy link
Member

@shaunanoordin shaunanoordin left a comment

Choose a reason for hiding this comment

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

PR Review

This PR bumps PJC to v 5.6.2, which packages #229 (dependabot bump), #235 (readme update), and, most functionally relevant, #240 (fix null items bug)

Testing

I tried testing this locally by doing a supposedly straightforward setup where...

  1. in my local panoptes-javascript-client folder, I'd pull this branch v5.6.2
  2. in my local Panoptes-Front-End folder (which uses PJC), I'd update the package.json so it imports my local PJC: "panoptes-client": "file:../panoptes-javascript-client"
  3. I'd run PFE, and make sure everything functions correctly

...but that didn't work out. There was some webpack issues resolving url.

So in the end, my less elegant setup was:

  1. pull branch v5.6.2 into my local panoptes-javascript-client folder
  2. copy the entire folder into my local PFE's modules, i.e. /panoptes-javascript-client ==> /Panoptes-Front-End/node_modules/panoptes-javascript-cient

Testing steps:

Tests look good, though I need to find a more elegant method of testing PJC updates on local.

Status

LGTM! 👍

@mcbouslog
Copy link
Contributor Author

Locally, I ran npm link in PJC then npm link panoptes-client in PFE. I think I got the same webpack error, and I think the solution was adding the forward slash to PFE's webpack.dev.js => url: require.resolve("url/"),. Then I got a few ENV errors, but in the same webpack.dev.js file changing to the following I think did the trick:

new webpack.EnvironmentPlugin({
      'HEAD_COMMIT': null,
      'NODE_ENV': 'development',
      'PANOPTES_API_APPLICATION': null,
      'PANOPTES_API_HOST': null,
      'STAT_HOST': null,
      'SUGAR_HOST': null,
      'TALK_HOST': null,
      'ERAS_HOST': null, // <--- this and the following are the new lines/changes
      'OAUTH_HOST': null,
      'REACT_APP_ENV': null,
    }),

I'm not sure if I should make these changes in PFE, note them somewhere in this library (README, local testing notes?), or otherwise.

@mcbouslog mcbouslog merged commit 8157794 into main Apr 14, 2024
3 checks passed
@mcbouslog mcbouslog deleted the v5.6.2 branch April 14, 2024 19:58
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.

2 participants