I had earlier talked about how I had planned to contribute to Telescope and even highlighted 2 issues that I'd picked up. After having worked on them, and picking up 2 new issues, I felt like this was the perfect time to give an update
Working on using cache from actions/setup-node #2537
After going through the cache documentation, I enabled the optional caching for each setup-node step in the workflows. For example:
- uses: actions/setup-node@v2.4.1
with:
node-version: '14'
cache: 'npm'
I also removed
uses: actions/cache@v2.1.6
and replaced it with setup-node caching instead. After a quick review, I pushed my changes and created a pull request to trigger the CI run on Github to test the cache:
Enabling cache for actions/setup-node - https://github.com/Seneca-CDOT/telescope/pull/2558
Having created the PR, I was awaiting the CI run result and to my dismay it failed:
Looking into the failure, I found out that runs were failing as there is no lock file (such as package-lock.json) in Telescope since the project is using save-exact. The built-in cache functionality in actions/setup-node had no option to use cache for projects without lock files (package-lock.json, yarn.lock).
To get around this limitation and respecting the save-exact rule of the project, I came up with a workaround to generate a lock file just for the CI run, to utilize the caching functionality.
Using npm, a package-lock.json can be updated (or generated in this case) using:
npm install --package-lock-only
So I added a precursor step to the workflows, to generate package-lock.json, after checking out the repository:
steps:
- uses: actions/checkout@v2
- name: Setup a lock file
run: npm install --package-lock-only
I pushed my changes to trigger a new run and the CI run partially passed, with the unit test jobs still failing
This was actually happening during the lock file generation step that I had earlier created:
After some investigation, it turned out that while npm install --package-lock-only did not download/install dependencies, it still ran the postinstall script. This meant that it was running run-s install:*, which was failing on the Windows run.
To fix this, I added a --ignore-scripts flag to the lock file generation step:
run: npm install --package-lock-only --ignore-scripts
All CI runs finally passed(actions run)and the cache was also working:
Since everything was now working, I requested a review, hoping to get this merged. However, after speaking to David, it turned out that a lot of the changes that I had made were going to be undone as there is an ongoing effort to switch to pnpm. Having spent a lot of time on this, I was a little disappointed by the PR being closed, but I had definitely learnt a lot working on this and overall it was a great learning experience, debugging and fixing the CI run failures.
Adding an anti-trojan source plugin [#2533]
For this issue, I needed to configure this anti-trojan source plugin with ESLint for Telescope and Satellite.
I installed it as a dev dependency using -save-dev and added it as a plugin in the ESLint config. As per the docs, I also added a setting to halt the run if a trojan source attack was found:
settings: {
...
'anti-trojan-source/no-bidi': 'error'
...
}
This was fairly straightforward and after ensuring that eslint worked, I created a pr:
Adding anti-trojan-source plugin to eslint - #2561
Having added it to Telescope, I switched to Satellite. To my surprise, ESLint was not configured for Satellite so there was no way for me to add this plugin. So, I filed this as a new issue and started working on it.
Add ESLint to Satellite - #2560
I started off by adding ESLint to the project. Since the project was Node based and used Jest, I installed jest as a plugin for ESLint and created the .eslintrc.js config file. Following on from the previous issue, I also included the anti-trojan source plugin and this is how the config file finally looked:
module.exports = {
env: {
node: true,
commonjs: true,
es2021: true,
jest: true,
},
extends: 'eslint:recommended',
parserOptions: {
ecmaVersion: 13,
},
plugins: ['anti-trojan-source', 'jest'],
rules: {
/**
* Halt if a trojan source attack is found
* https://github.com/lirantal/eslint-plugin-anti-trojan-source
*/
'anti-trojan-source/no-bidi': 'error',
},
};
After configured ESLint, I added scripts to package.json to make it easier to lint the code:
"eslint": "eslint --config .eslintrc.js \"**/*.js\"",
"eslint-fix": "eslint --config .eslintrc.js \"**/*.js\" --fix",
"lint": "npm run eslint",
It was now time to run ESLint, and I expected some linting errors to pop up, which they did! There were some imports missing and some ununsed variables which I quickly went over and fixed. The ./test.js file specifically had a lot of no-unused-vars errors such as Such as (req,user) => ...and I ended up adding a override in the config to ignore them as while the variables were unused, they did convey meaning.
overrides: [
{
files: ['**/test.js'],
rules: {
'no-unused-vars': 'off',
},
},
],
The project was already (husky)[https://www.npmjs.com/package/husky] to run prettier with a pre-commit hook. Since I had now added ESLint, I also included a linting step:
"pre-commit": "pretty-quick --staged && npm run lint"
After some testing to ensure that everything worked, I then created a pull request and it is currently under review:
Add ESLint to satellite along with anti trojan plugin - #21
While I wait for this to be merged, I've also picked up another issue in Telescope. Tying into my goal of improving the developer experience working on the different tools and technologies being used in the project, this one is related to gitpod:
Optimize Gitpod Environment Image #2564
Currently the project is using the default gitpod/workspace-full image, which includes a lot of tools that are not needed. The goal is to transition to using the smaller gitpod/workspace-base image and this is the next issue I'm going to work on!
Top comments (0)