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.

Sign up for GitHub

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

Jump to bottom

When running npm run build-node dist/indexeddbshim-node.js changes #291

Closed
erikvold opened this issue Feb 22, 2017 · 6 comments
Closed

When running npm run build-node dist/indexeddbshim-node.js changes #291

erikvold opened this issue Feb 22, 2017 · 6 comments
Assignees
@brettz9
Labels
Releases Testing

Comments

@erikvold
Copy link
Contributor

erikvold commented Feb 22, 2017 •
edited

When I clone this repo, run npm install, then npm run build-node then git status will output:

	modified:   dist/indexeddbshim-node.js
	modified:   dist/indexeddbshim-node.min.js
	modified:   dist/indexeddbshim-node.min.js.map

this is the diff master...erikvold:run-build-node

shouldn't there be no change?

Using node 7.5.0 npm 4.1.2
@erikvold
Copy link
Contributor Author

@brettz9 ping

@erikvold erikvold mentioned this issue Feb 22, 2017
Merged
@brettz9
Copy link
Collaborator

brettz9 commented Feb 22, 2017 •
edited

I'll try to start over, removing my node_modules and reinstalling as well as incorporate your changes and then retest.

Btw, I'm still pretty actively working on things (though ATM working on our new dependency, typeson-registry), trying to clear up or address the remaining W3C failing tests, so though I try ensuring everything is sound (at least with the W3C tests though often all tests) before pushing to master, there ought to be more fixes on the way in the shorter term and more complete testing when that is all complete.

But yours ought to be a very helpful fix to have in the interim especially to the extent it is blocking others from testing.

@brettz9
Copy link
Collaborator

brettz9 commented Feb 23, 2017 •
edited

Could you try this again for me? I wonder whether the recent changes in master should fix this.

@erikvold
Copy link
Contributor Author

erikvold commented Feb 28, 2017 •
edited

I updated to master removed the node_modules subdirectory and ran npm install && npm run build-node again and got master...erikvold:run-build-node-ii

brettz9 added a commit to brettz9/IndexedDBShim that referenced this issue Feb 28, 2017
@brettz9
INCOMPLETE: Need to check logic for checking/incrementing/returning key
cc998eb 
   as well as properly handle numbers just beyond the max allowed by ES
   since confuable if only incrementing by one
- Fix: As per new requirements, ensure numbers higher than the max
    (including non-finite) have the effect of setting the current number
    to the max
- Fix: Default to `global` where no `window` or `self` is defined
- Refactoring (SQL): (avoid unused column name in SELECT)
- Refactoring (spec): Make current number retrieval routines more closely
   parallel spec
- npm: Update ESLint and compat plugin; add shrinkwrap to better ensure
   consistency across installs (and for testing consistency)
- Grunt: Try stabilizing builds for issue indexeddbshim#291
brettz9 added a commit to brettz9/IndexedDBShim that referenced this issue Feb 28, 2017
@brettz9
INCOMPLETE: Need to check logic for checking/incrementing/returning key
0643f7d 
   as well as properly handle numbers just beyond the max allowed by ES
   since confuable if only incrementing by one; add to CHANGES
- Fix: As per new requirements, ensure numbers higher than the max
    (including non-finite) have the effect of setting the current number
    to the max
- Fix: Default to `global` where no `window` or `self` is defined
- Refactoring (SQL): (avoid unused column name in SELECT)
- Refactoring (spec): Make current number retrieval routines more closely
   parallel spec
- npm: Update ESLint and compat plugin; add shrinkwrap to better ensure
   consistency across installs (and for testing consistency)
- Grunt: Try stabilizing builds for issue indexeddbshim#291
brettz9 added a commit to brettz9/IndexedDBShim that referenced this issue Feb 28, 2017
@brettz9
INCOMPLETE: Need to check logic for checking/incrementing/returning key
0065ef2 
   as well as properly handle numbers just beyond the max allowed by ES
   since confuable if only incrementing by one; add to CHANGES
- Fix: As per new requirements, ensure numbers higher than the max
    (including non-finite) have the effect of setting the current number
    to the max
- Fix: Default to `global` where no `window` or `self` is defined
- Refactoring (SQL): (avoid unused column name in SELECT)
- Refactoring (spec): Make current number retrieval routines more closely
   parallel spec
- npm: Update ESLint and compat plugin; add shrinkwrap to better ensure
   consistency across installs (and for testing consistency)
- Grunt: Try stabilizing builds for issue indexeddbshim#291
brettz9 added a commit to brettz9/IndexedDBShim that referenced this issue Mar 1, 2017
@brettz9
INCOMPLETE: Need to check logic for checking/incrementing/returning key
02efe88 
   as well as properly handle numbers just beyond the max allowed by ES
   since confuable if only incrementing by one; add to CHANGES
   Check working? - Fix: Support `Blob`/`File`/`FileList` in SCA via
        `typeson-registry`
- Fix: As per new requirements, ensure numbers higher than the max
    (including non-finite) have the effect of setting the current number
    to the max
- Fix: Default to `global` where no `window` or `self` is defined
- Refactoring (SQL): (avoid unused column name in SELECT)
- Refactoring (spec): Make current number retrieval routines more closely
   parallel spec
- npm: Update ESLint and compat plugin; add shrinkwrap to better ensure
   consistency across installs (and for testing consistency)
- Grunt: Try stabilizing builds for issue indexeddbshim#291
brettz9 added a commit to brettz9/IndexedDBShim that referenced this issue Mar 1, 2017
@brettz9
INCOMPLETE: Need to check logic for checking/incrementing/returning key
4cdf752 
   as well as properly handle numbers just beyond the max allowed by ES
   since confuable if only incrementing by one; add to CHANGES
   Check working? - Fix: Support `Blob`/`File`/`FileList` in SCA via
        `typeson-registry`
- Fix: As per new requirements, ensure numbers higher than the max
    (including non-finite) have the effect of setting the current number
    to the max
- Fix: Default to `global` where no `window` or `self` is defined
- Refactoring (SQL): (avoid unused column name in SELECT)
- Refactoring (spec): Make current number retrieval routines more closely
   parallel spec
- npm: Update ESLint and compat plugin; add shrinkwrap to better ensure
   consistency across installs (and for testing consistency)
- Grunt: Try stabilizing builds for issue indexeddbshim#291
@brettz9
Copy link
Collaborator

brettz9 commented Mar 2, 2017 •
edited

Thanks for checking... Hmm... I still want to take a closer look. When done with a current PR, I may add npm shrinkwrap to ensure we have all the same dependencies. I also saw that such an issue with varying browserify order if basedir were not supplied so I might try setting that and see if that may resolve any other such relative path problems.

@brettz9 brettz9 assigned brettz9 and unassigned erikvold Mar 2, 2017
@brettz9
Copy link
Collaborator

brettz9 commented Mar 7, 2017

Looking at the diff for your run-build-node-ii branch, it looks as expected--it includes some more recent updates to the typeson repo. (Our references to particular repos in npm (e.g., master branch) without a version may turn up different code for now. Though I have commit access to those repos (the current ones are my forks), I am not ready to either stabilize on a version and push back upstream or publish my own fork, so it may be a little inconsistent for now though. However, it is part of my final proposed goals for version 3.0.0 at #262 (comment) to specify stable versions, and on my next PR, I'll even try to remember to shrinkwrap it so that will enforce stable versions for the time-being to facilitate testing such as you are doing.)

@brettz9 brettz9 closed this as completed Mar 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@brettz9 brettz9

Labels
Releases Testing
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@brettz9 @erikvold