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

JS refactoring #2408

Merged

Conversation

danielamorse
Copy link
Collaborator

@danielamorse danielamorse commented Dec 17, 2021

Jira

n/a

Summary

Refactors some JS in #2406.

Details

  • Remove semver-compare dependency. The semver package we already use has a function for comparing.
  • Get lerna.json using process.cwd() rather than relative path from package.
  • Refactor a couple JS loops to use ES6 methods that are shorter and do the same thing.
  • Use semver validation instead of RegEx to filter out tags. Regex works just fine here, but I thought the semver validation would be a more explicit check for what we're looking for, and I prefer to use RegEx only as a last resort.

@github-actions github-actions bot added the type: feature List this PR in the 'Features' section of the release notes. label Dec 17, 2021
@colbytcook colbytcook temporarily deployed to feature/prod-dropdown-cleanup--edits--branch-preview December 17, 2021 22:45 Inactive
@colbytcook colbytcook temporarily deployed to feature/prod-dropdown-cleanup--edits--branch-preview December 17, 2021 23:13 Inactive
let versionSpinner;

const { getConfig } = require('@bolt/build-utils/config-store');
const { fileExists } = require('@bolt/build-utils/general');
const lernaConfig = require('./../../../../lerna.json');
const lernaConfig = require(path.join(process.cwd(), '../lerna.json'));
Copy link
Contributor

Choose a reason for hiding this comment

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

@danielamorse I should I mentioned this but the path.join fails depending on what command is run. the ../ works on yarn start but fails on yarn build (I think) due the directory changing to be the parent folder of the Bolt repository. We might want to turn this back the ugly relative link.

Edit: the command that fails is the yarn test:js

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Makes sense. I reverted my change.

@colbytcook colbytcook self-requested a review December 20, 2021 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature List this PR in the 'Features' section of the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants