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

[new] add leaflet-plugins@1.3.12 with git auto-update #6149

Closed
wants to merge 1 commit into from

Conversation

Amomo
Copy link
Contributor

@Amomo Amomo commented Nov 6, 2015

Hi @Piicksarn

For #6137, I add leaflet-plugins at v1.3.12 and add git auto-update config in its package.json.
repo: https://github.com/shramov/leaflet-plugins

As README.md and bower.json said, I add Vector layers (layer/vector/), Providers (layer/tile) , (layer/) and control plugins (control/).

This library contains plugins of leaflet, so it don't has a main file.
Randomly, I pick "layer/vector/OSM.min.js" for filename field.

Would you mind checking this PR for me? Thank you~ :-D

  • Not found on cdnjs repo
  • No already exist issue and PR
  • Notable popularity : Watch 54, Star 440, Fork 206
  • Project has public repository on famous online hosting platform (or been hosted on npm)
  • Has valid tags for each versions (for git auto-update)
  • Auto-update setup
  • Auto-update target/source is valid.
  • Auto-update filemap is correct.

@Amomo Amomo changed the title add leaflet-plugins@1.3.12 with git auto-update [new] add leaflet-plugins@1.3.12 with git auto-update Nov 6, 2015
@Piicksarn
Copy link
Contributor

It's fine!
@PeterDaveHello, would you please have a look at the pr?

the main files shown in bower.json :

"main": [
    "control/Distance.js",
    "control/Layers.Load.js", 
    "control/Permalink.js",
    "control/Permalink.Layer.js",
    "control/Permalink.Line.js",
    "control/Permalink.Marker.js",
    "layer/Icon.Canvas.js",
    "layer/Layer.Deferred.js",
    "layer/Marker.Rotate.js",
    "layer/Marker.Text.js",
    "layer/OpenStreetBugs.js",
    "layer/vector/GPX.js",
    "layer/vector/GPX.Speed.js",
    "layer/vector/KML.js",
    "layer/vector/OSM.js",
    "layer/tile/Bing.js",
    "layer/tile/Google.js",
    "layer/tile/Yandex.js"
]

@Piicksarn Piicksarn assigned PeterDaveHello and unassigned Piicksarn Nov 8, 2015
@PeterDaveHello
Copy link
Contributor

hmmmm ... I would like to ask author to confirm that.

@shramov would you please give us some advice if we wanna host this lib on the cdn? Thanks.

@Amomo
Copy link
Contributor Author

Amomo commented Nov 22, 2015

Hi @brunob would you please give us some advice if we wanna host this lib on the cdn?
Which files should we add? Is there a main file we could point to?
Thank you! :)

@brunob
Copy link

brunob commented Nov 22, 2015

@Amomo there is no main file, each script is independent, the scripts listed in bower file seems ok for me.

@Amomo
Copy link
Contributor Author

Amomo commented Nov 22, 2015

@brunob thanks for your response! :-D

@Amomo Amomo force-pushed the leaflet-plugins branch 2 times, most recently from cf3edf4 to 956885c Compare November 25, 2015 16:40
@Amomo
Copy link
Contributor Author

Amomo commented Nov 25, 2015

@Piicksarn I have rebased this PR, could you check it again? Thank you!

@Amomo Amomo assigned Piicksarn and unassigned PeterDaveHello Nov 25, 2015
@Piicksarn
Copy link
Contributor

@PeterDaveHello, I think the pr is fine!
Would you mind to have a look at the pr?
Thank you!

@PeterDaveHello
Copy link
Contributor

Duplicated file map ...

@Amomo
Copy link
Contributor Author

Amomo commented Nov 26, 2015

@PeterDaveHello sorry, I am not sure what does your "Duplicated file map" means.
The library doesn't provide minified files, so I note the needed minified files in "requiredFiles" field.

Or, could you please point out which part I did it wrong? Thank you!

"control/Permalink*.js",
"control/Permalink.Layer*.js",
"control/Permalink.Line*.js",
"control/Permalink.Marker*.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't "control/Permalink*.js" include "control/Permalink.Layer*.js"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes! Thanks a lot.

@Amomo
Copy link
Contributor Author

Amomo commented Dec 11, 2015

@Piicksarn Would you mind to check this PR for me? Thank you~ :-)

@PeterDaveHello
Copy link
Contributor

@LeaYeh @maruilian11 would you please also help review this PR? Thanks.

@Piicksarn
Copy link
Contributor

@PeterDaveHello, the pr is fine.
would you please have a look at the pr?
thanks.

@maruilian11
Copy link
Contributor

@PeterDaveHello i think the pr is ok.

@PeterDaveHello
Copy link
Contributor

2 days ago ...

@maruilian11
Copy link
Contributor

@Amomo could you rebase the commits?

@Amomo
Copy link
Contributor Author

Amomo commented Dec 15, 2015

@maruilian11 @PeterDaveHello @Piicksarn thank you.
I have rebase on latest master branch.

@maruilian11
Copy link
Contributor

@Amomo sorry... could you rebase it again 😞

@Amomo
Copy link
Contributor Author

Amomo commented Dec 21, 2015

@maruilian11 I have updated it to v1.4.2, could you help me check this PR again?
Thank you~ :-D

@maruilian11
Copy link
Contributor

@Amomo sorry..again😭

@Amomo
Copy link
Contributor Author

Amomo commented Dec 25, 2015

@maruilian11 Could you help me check it again? Thank you~ :-D

@maruilian11
Copy link
Contributor

@PeterDaveHello i think this PR is ok.

@PeterDaveHello
Copy link
Contributor

@Piicksarn @LeaYeh please help double check this, thanks.

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.

None yet

6 participants