sindresorhus / gulp-rev

Static asset revisioning by appending content hash to filenames: `unicorn.css` → `unicorn-d41d8cd98f.css`
MIT License
1.54k stars 218 forks source link

Version in GET param #198

Closed d-u-a-l closed 7 years ago

d-u-a-l commented 7 years ago

Added an option not to modify filenames, but change GET parameter (for example 'v', it could be made changeable through options) like common.css?v=HASH. In this case you don't need to make rev() call to change filenames (and by that you don't have to worry about deleting old files), just rev.manifest({get:true})

mention-bot commented 7 years ago

@d-u-a-l, thanks for your PR! By analyzing the history of the files in this pull request, we identified @c0, @borisceranic and @sindresorhus to be potential reviewers.

benbieler commented 7 years ago

Sorry, but these are the node versions we are currently supporting:

node_js:
  - 'iojs'
  - '0.12'
  - '0.10'

@sindresorhus we really need to upgrade...

sindresorhus commented 7 years ago

Duplicate of https://github.com/sindresorhus/gulp-rev/issues/129.

bobthecow commented 7 years ago

This is asking to put the rev hash in a query param, not asking to use the semver version, so it's not a dupe of #129

@d-u-a-l I would highly recommend against this. And Steve Souders says it's a bad idea :)

d-u-a-l commented 7 years ago

@benbieler I don't understand, why you are telling me about Node versions, I didn't write any modern code, just plain old JS ES5. It's not very pretty, but it's my first pull-request, so don't judje to hard :)

@sindresorhus No, as @bobthecow pointed, it's not a duplicate of #129. My suggestion was to give users a choise to put hashes not into filename (thus, changing names completely), but into query parameter - so, you don't have to manually delete old files (or use yet another plugins to do this). It simplifyes the use of a plugin, because you have to only call rev.manifest function ('cause rev is for renaming)

@bobthecow I suggested to use it only as an option, to give users choise between changing resource name and only changing a query parameter in html. Just because some proxy providers can't (or don't want to) configure their servers as they should, doesn't mean, that we should refuse of using this technique

sindresorhus commented 7 years ago

This plugin is feature complete. We're not adding more options to it.

d-u-a-l commented 7 years ago

@sindresorhus you don't need to make excuses like a "duplicate" or another one. You don't want to make changes I suggested, it's your right, just say so. For now I'm OK with things as they are, maybe in future I'll publish my own plugin, which does just what I need. If it'll contain code, similar to yours, it'll just be a weird coincidence :D

sindresorhus commented 7 years ago

you don't need to make excuses like a "duplicate" or another one.

That was not an excuse. I mistakenly thought it was a duplicate. I get hundreds of duplicates and low quality issues every week that I need to handle. Once in awhile, a mistake slips through.

benbieler commented 7 years ago

@d-u-a-l Your build was failing due to ES6 stuff. Of course I know that this has nothing to do with your changes. It was another module.