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

Bug 1686678 - Prepare Glean.js to publish for web extensions #48

Closed
wants to merge 4 commits into from

Conversation

brizental
Copy link
Contributor

@brizental brizental commented Feb 11, 2021

For the Glean.js web extension package I could only think of the name @mozilla/glean-webext, but I don't like it.

Pls, help.

For reference, these are the logs of a publishing dry-run that I tested using the npm run publish:webext command:

npm notice 
npm notice 📦  @mozilla/glean-webext@0.1.0
npm notice === Tarball Contents === 
npm notice 16.7kB LICENSE                         
npm notice 58.4kB dist/glean.js                   
npm notice 2.8kB  package.json                    
npm notice 2.1kB  CHANGELOG.md                    
npm notice 1.2kB  README.md                       
npm notice 1.8kB  src/metrics/types/boolean.ts    
npm notice 2.1kB  src/upload/uploader/browser.ts  
npm notice 1.8kB  src/config.ts                   
npm notice 1.6kB  src/constants.ts                
npm notice 3.6kB  src/metrics/types/counter.ts    
npm notice 9.1kB  src/metrics/database.ts         
npm notice 5.3kB  src/pings/database.ts           
npm notice 10.0kB src/metrics/types/datetime.ts   
npm notice 12.3kB src/dispatcher.ts               
npm notice 3.3kB  src/metrics/types/event.ts      
npm notice 5.9kB  src/metrics/events_database.ts  
npm notice 14.9kB src/glean.ts                    
npm notice 2.7kB  src/index.ts                    
npm notice 4.3kB  src/metrics/index.ts            
npm notice 2.8kB  src/pings/index.ts              
npm notice 1.7kB  src/platform_info/index.ts      
npm notice 2.5kB  src/storage/index.ts            
npm notice 496B   src/storage/persistent/index.ts 
npm notice 10.1kB src/upload/index.ts             
npm notice 1.8kB  src/upload/uploader/index.ts    
npm notice 5.3kB  src/internal_metrics.ts         
npm notice 938B   src/internal_pings.ts           
npm notice 7.7kB  src/pings/maker.ts              
npm notice 2.9kB  src/metrics/types/string.ts     
npm notice 764B   src/metrics/time_unit.ts        
npm notice 1.9kB  src/metrics/utils.ts            
npm notice 3.7kB  src/storage/utils.ts            
npm notice 4.5kB  src/utils.ts                    
npm notice 3.2kB  src/metrics/types/uuid.ts       
npm notice 1.6kB  src/storage/weak.ts             
npm notice 1.7kB  src/platform_info/webext.ts     
npm notice 5.0kB  src/storage/persistent/webext.ts
npm notice 8.2kB  src/metrics.yaml                
npm notice 1.0kB  src/pings.yaml                  
npm notice === Tarball Details === 
npm notice name:          @mozilla/glean-webext                   
npm notice version:       0.1.0                                   
npm notice package size:  56.7 kB                                 
npm notice unpacked size: 227.8 kB                                
npm notice shasum:        b820d0120e8528e89469453d1d349916534ee42d
npm notice integrity:     sha512-5lQG+ZyKRmPJX[...]Fe4bFVF0mCrVw==
npm notice total files:   39                                      
npm notice 

command: npm install
- run:
name: NPM Authentication
command: echo "//registry.npmjs.org/:_authToken=${NPM_TOKEN}" > .npmrc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to remember to set this environment variable before tagging for the first time. I copied this code from the nimbus-shared CI config, so I trust it works.

Copy link
Contributor

Choose a reason for hiding this comment

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

@badboy also has expertise here :)

@@ -73,6 +88,15 @@ workflows:
- lint
- unit-tests
- check-qt-js
- publish-webext:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is also copied from the glean_parser publishing CI task.

"build:test-webext": "cd tests/utils/webext/sample/ && npm install && npm run build:xpi",
"prepublish:webext": "npm run build:webext && json -I -f package.json -e \"this.name=\\\"@mozilla/glean-webext\\\"\"",
"publish:webext": "npm run prepublish:webext && npm publish",
"postpublish": "json -I -f package.json -e \"delete this.name\""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit weird, I know. But it is necessary since we plan to publish different packages from this same package.json. Note that I set the correct name on the prepublish command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I will need to change the glean_parser template, because right now it just assumes the package will be called "glean". That was an oversight on my part. Apologies.

@brizental brizental requested a review from Dexterp37 February 11, 2021 11:57
@Dexterp37
Copy link
Contributor

For the Glean.js web extension package I could only think of the name @mozilla/glean-webext, but I don't like it.

Mh, that's a good point. I don't really have any good input for this. Maybe check in the channel :)

For reference, these are the logs of a publishing dry-run that I tested using the npm run publish:webext command:

npm notice 
npm notice 📦  @mozilla/glean-webext@0.1.0
npm notice === Tarball Contents === 
npm notice 16.7kB LICENSE                         
npm notice 58.4kB dist/glean.js                   
npm notice 2.8kB  package.json                    
npm notice 2.1kB  CHANGELOG.md                    
npm notice 1.2kB  README.md              
...
npm notice 1.8kB  src/ ...

Do we need to publish things in src/?

@brizental
Copy link
Contributor Author

Do we need to publish things in src/?

Eh, you are right. No we don't.

@brizental
Copy link
Contributor Author

Closing this in favour of #73

@brizental brizental closed this Feb 16, 2021
@brizental brizental deleted the 1686678-ship-it branch February 16, 2021 14:54
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