

My First Open Source Project: A Two Week Chrome Extension - joshholat
http://joshholat.com/blog/my-first-open-source-project-and-a-two-week-chrome-extension/

======
WesleyJohnson
Kudos for "shipping" something. That is the hardest part sometimes. As per
your request for feedback (though I wouldn't say I'm smarter than you), some
thoughts:

1) The link to your github repo is broken on your blog post.

2) In your manifest.json, you're setting permissions to "tabs",
"www.google.com", "<http://*/*> and "<https://*/*>. If I'm not mistaken, the
urls listed in the permissions field are for urls you'd like your extension to
be able to call out to when doing AJAX requests, posts, etc. Enabling your app
to make cross-site request to every domain, when you really only need
google.com for the calendar, might throw up a red flag to more security-
conscious users.

3) You're injecting a blank css document into every url the user visits. I
would remove this unless you're planning on adding some styles into the css
file.

4) The only other thing I see is more preference: You're doing a lot of work
with the browser's dom api to create elements in your content script. Since
you're already including jQuery in your extension, it would be trivial to
inject the jQuery.js file into the page via your content script and use the
power of jQuery to create your dom elements in far less code. Of course, there
is nothing wrong with doing it in using raw JavaScript since it's targeted at
a single browser and you don't need to worry about being cross browser
compatible.

Thanks for posting, I'm always curious to see how other extensions are put
together and might take away some ideas on interacting with google services.

~~~
joshholat
WesleyJohnson,

Thanks for the feedback!

1) The link should be fixed now. Thanks for informing me about that. I think I
had a return character in it accidentally.

2) Okay, that makes sense. I think you are correct because when the user
installs the extension it says something along the lines of needing access to
basically any data. Obviously, my extension wouldn't need all that data. That
must be the issue then.

3) Ahh, right. I had some stuff in there at one point but then decided to not
use it. Must've forgot to delete it.

4) I had thought during the process that there was probably a better way to do
it. However, for sake of just getting something out there I've left this code
as is. Believe it or not, it was worse at one point. I'll look into other ways
of doing it, though.

Thanks again.

------
Timothee
I've noticed that you didn't put any open-source license with your code, which
is understandable since it's your first. :)

So my question (likely to others) is this: when you do that kind of week-end
projects, do you put a license with it and if so, which one?

The reason I'm asking is I also put some code on GitHub from little projects,
but without any license for now, mostly because I didn't really care either
way. But I imagine it'd make sense to know about a few good ones to go with
and use that for most cases. (for example, it seems to me that the MIT license
has got some popularity over the GPL, but I'm not sure why)

~~~
Macha
MIT vs GPL in a nutshell:

MIT: here's my code, you can do what you want, just don't be a dick.

GPL: here's my code. If you use it to build something on top of, your code
must be open source two. And here's a long list of ways of bypassing that that
aren't allowed.

------
jessor
Very nice, this is quite helpful. Thank you!

I just noticed this on last.fm, you may want to add z-index: 9999; to your
"popOver" styles to fix problems where sites give their logos a z-index. Also,
I'm missing a close/cancel button. Lastly, adding support for last.fm's format
("Saturday 22 January 2011") would be sweet.

~~~
joshholat
Thanks for the tips. If you look in the README on the code, you'll see that
two of those actually fall under stuff I'm currently working on. Also, I'll
add that date format shortly.

------
sayrer
random code comments on content_script.js:

The indentation is crazy (subjective). Use spaces not tabs, choose 2 or 4
spaces. I prefer two, but don't really care.

there's a lot of repetition: All of the textboxes could be initialized with a
function that took key/value pairs for attributes. They are all set to float
right.

don't write stuff like document.getElementById("popOver") over and over, it's
ugly. Do it once and assign it to a local variable. This is for style, not
efficiency.

I didn't read the getTextSelection() function thoroughly, but in my
experience, stuff that looks like that never works quite right. Attacking date
inputs is tough, and will probably require a separate, tested library.

~~~
joshholat
Sayrer,

"The indentation is crazy (subjective)." - I tend to like to use tabs just
because it's easier on the eyes to see how code blocks differentiate and I've
grown accustomed to hitting tab rather than space. Perhaps I'll try that out,
though.

"there's a lot of repetition" - Right, I did that as kind of the quick and
dirty way to get it up. I will most likely end up changing that as per your
suggestion. Thanks.

"Attacking date inputs is tough, and will probably require a separate, tested
library." - You're right, it's not very fun. Thanks for the suggestion, I
think this should work a lot better for me: <http://www.datejs.com/>

~~~
DTrejo
Your editor should have a "soft tabs" option, which allows you to edit as
normal, but two spaces are inserted instead of a tab ('\t').

~~~
joshholat
I'll look into that, thanks.

