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

Update README #217

Merged
merged 3 commits into from
Mar 7, 2024
Merged

Update README #217

merged 3 commits into from
Mar 7, 2024

Conversation

MatthijsSmets
Copy link
Contributor

Angular.json was updated in ladybug-frontend which requires an update to the README.md

@MatthijsSmets MatthijsSmets requested a review from jacodg March 5, 2024 10:13
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@philipsens philipsens left a comment

Choose a reason for hiding this comment

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

It's hard to review the changes will all the changed syntax and indent.

I'd suggest to put it back.

@@ -240,12 +240,8 @@ Create and publish NPM package and WebJar
- In the terminal run the following command `npm login`, which prompts you to log into NPM.

### Creating and publishing an NPM package
- Run the command `yarn install --frozen-lockfile`, to sync with changes done by others
- Run the command `yarn install --immutable`, to sync with changes done by others
- Run the command `ng build`, to build the current project
Copy link
Member

Choose a reason for hiding this comment

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

I have to do yarn ng build instead of just ng build.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That means your angular cli is not installed properly, this still works for anyone that has it installed correctly.
This is also something that was not changed in this pr.

README.md Outdated
- Copy the following files into the generated `dist/ladybug` folder
- README.md
- LICENSE
- package.json
- In the `dist/ladybug/index.html` change the types of the three scripts on line 15 to `application/javascript` (so `type="module"` -> `type="application/javascript"`). The specific scripts are:
Copy link
Member

Choose a reason for hiding this comment

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

On my computer it turned out to be line 14.

Copy link
Member

@mhdirkse mhdirkse left a comment

Choose a reason for hiding this comment

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

You need npm publish -otp=<code from authenticator app of your telephone>

@MatthijsSmets MatthijsSmets requested a review from philipsens March 5, 2024 17:16
@MatthijsSmets
Copy link
Contributor Author

You need npm publish -otp=<code from authenticator app of your telephone>

This is not the case for every user, it depends on if you have 2FA enabled.

@MatthijsSmets MatthijsSmets dismissed mhdirkse’s stale review March 6, 2024 11:07

Suggested changes are not required

@jacodg jacodg merged commit 1885bf7 into master Mar 7, 2024
1 check passed
@jacodg jacodg deleted the update-read-me branch March 7, 2024 16:46
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.

4 participants