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

Updated Webpack Config Files & Add Citation File #695

Merged
merged 8 commits into from
Jul 11, 2023

Conversation

adithyaakrishna
Copy link
Contributor

Description:

  • This PR updated webpack config to show error details and also has some linting updates.
  • I have also added a Citation.cff file such that users can cite this repository in a easier way.

More Info: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-citation-files

The citation file adds something like this on the side menu

Screenshot 2023-07-10 at 12 23 42 AM Screenshot 2023-07-10 at 12 24 16 AM

Adithya Krishna added 2 commits July 9, 2023 18:31
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
@adithyaakrishna
Copy link
Contributor Author

I have created the citation file to the best of my knowledge, please do let me know if there are any other changes required to it 😶

Adithya Krishna added 2 commits July 9, 2023 19:04
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Comment on lines 20 to 27
- name: Install Dependencies
# As our package has a postInstall script, we dont need a explicit job for that
- name: Install & Build Dependencies
run: npm install

- name: Build 3Dmol.js
run: |
npm run build:dev
npm run build:prod
npm run generate:tests
Copy link
Contributor Author

@adithyaakrishna adithyaakrishna Jul 9, 2023

Choose a reason for hiding this comment

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

Removed the Build job, which saves us ~56 seconds in terms of combined CI time wrt CodeQL and Build Workflows

But why this change? Our package.json has a "postInstall" script here, https://github.com/3dmol/3Dmol.js/blob/master/package.json#L33 which runs npm run build upon installation of dependencies. Till now we were running a redundant job, hence removed the same

postInstall running after installation of dependencies
Screenshot 2023-07-10 at 12 43 23 AM

With Build Job = +28 seconds - Which was Duplicated by the above postInstall script
Screenshot 2023-07-10 at 12 42 34 AM

Signed-off-by: Adithya Krishna <[email protected]>
@codecov
Copy link

codecov bot commented Jul 9, 2023

Codecov Report

Merging #695 (3c43b43) into master (de25047) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 3c43b43 differs from pull request most recent head b726359. Consider uploading reports for the commit b726359 to get more accurate results

@@           Coverage Diff           @@
##           master     #695   +/-   ##
=======================================
  Coverage   83.01%   83.01%           
=======================================
  Files         141      141           
  Lines       11450    11450           
  Branches     2112     2112           
=======================================
  Hits         9505     9505           
  Misses       1613     1613           
  Partials      332      332           
Impacted Files Coverage Δ
src/utilities.ts 74.65% <100.00%> (+1.04%) ⬆️

... and 3 files with indirect coverage changes

Adithya Krishna added 3 commits July 10, 2023 04:18
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
@@ -1,4 +1,3 @@
/* eslint-disable no-undef*/
Copy link
Contributor

Choose a reason for hiding this comment

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

This is here for a reason - so VS code doesn't report errors where there are none.

@dkoes dkoes merged commit cc194e1 into 3dmol:master Jul 11, 2023
5 checks passed
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