Skip to content

Conversation

omidb
Copy link
Contributor

@omidb omidb commented May 20, 2015

adding Literal object construction using an scala object interface section to address http://stackoverflow.com/questions/26638171/how-do-i-create-options-objects-in-scala-js

Fix #152: Document option object pattern

Copy link
Contributor

Choose a reason for hiding this comment

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

I would make this a title of lower order (###).

Copy link
Contributor

Choose a reason for hiding this comment

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

"Scala" with capital first letter

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, consider making this section more about JS facades. Maybe even make the title: "Option objects for native JavaScript libraries"

@gzm0
Copy link
Contributor

gzm0 commented May 20, 2015

Thanks for the contribution.

@gzm0
Copy link
Contributor

gzm0 commented May 20, 2015

Please reference the issue in your commit message: "Fix #152: Document option object pattern". This will automatically establish the link between issue and PR.

@gzm0
Copy link
Contributor

gzm0 commented May 20, 2015

(this might also be a better commit title than " Update calling-javascript.md". git is able to tell me which files were altered. We don't need to duplicate that information in the commit message).

@gzm0
Copy link
Contributor

gzm0 commented May 20, 2015

That's all.

@omidb
Copy link
Contributor Author

omidb commented May 20, 2015

thanks for all comments.

@omidb omidb changed the title Update calling-javascript.md Fix #152: Document option object pattern May 20, 2015
@omidb
Copy link
Contributor Author

omidb commented May 21, 2015

@gzm0 I believe, I've addressed all of your points in 550d61c

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add an empty line?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, Scala upper case, please.

Copy link
Contributor

Choose a reason for hiding this comment

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

And "A" instead of "An" ("An" only if followed by a vovel).

Copy link
Contributor

Choose a reason for hiding this comment

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

Also maybe say "should" instead of "can". And replace "easy access" (we don't really access, do we) by "type safe creation". (this is why one should do this, rather than scattering js.Dynamic.literal all over the code).

@gzm0
Copy link
Contributor

gzm0 commented May 21, 2015

That's all. Once you are done, please squash the commits into a single commit and give it the PR's title.

@omidb
Copy link
Contributor Author

omidb commented May 21, 2015

@gzm0 I don't have that much experience using Github, I was using the online editor and have no idea how can I squash the commits into a single commit!

@sjrd
Copy link
Member

sjrd commented May 21, 2015

@omidb You'll have to do that using a local git client. After having cloned your fork, cd into its directory, then:

$ git remote add scalajs https://github.com/scala-js/scala-js-website.git 

and then you can rebase like this:

$ git checkout patch-1 $ git fetch scalajs $ git rebase -i scalajs/master 

Now in the editor that spawns, replace pick by squash (or just s) for all but the first line. Then save and quit the editor. The rebasing proceeds, and a new editor spawns to let you edit the commit message for the squashed commits. Do so (with the PR's title), then save and quit.
You can now force-push the branch:

$ git push -f origin patch-1 
adding `Literal object construction using an scala object interface` section to address http://stackoverflow.com/questions/26638171/how-do-i-create-options-objects-in-scala-js Fix #152: Document option object pattern: option patterns has been documented in this fix.
@omidb
Copy link
Contributor Author

omidb commented Jun 15, 2015

@sjrd thanks for detailed instructions, I did it

@ochrons
Copy link
Contributor

ochrons commented Nov 27, 2015

@omidb care to update this for the new site?

@omidb
Copy link
Contributor Author

omidb commented Nov 27, 2015

@ochrons sure, should I do anything? (I barely remember what happen :D )

@ochrons
Copy link
Contributor

ochrons commented Nov 27, 2015

As things have changed and moved around quite a lot, it might be easiest to just make a new PR with the same content on the current document :)

omidb added a commit to omidb/scala-js-website that referenced this pull request Nov 27, 2015
From a PR for last website scala-js#153 adding Literal object construction using an scala object interface section to address http://stackoverflow.com/questions/26638171/how-do-i-create-options-objects-in-scala-js Fix scala-js#152: Document option object pattern
omidb added a commit to omidb/scala-js-website that referenced this pull request Nov 28, 2015
From a PR for last website scala-js#153 adding Literal object construction using an scala object interface section to address http://stackoverflow.com/questions/26638171/how-do-i-create-options-objects-in-scala-js Fix scala-js#152: Document option object pattern
omidb added a commit to omidb/scala-js-website that referenced this pull request Nov 28, 2015
From a PR for last website scala-js#153 adding Literal object construction using an scala object interface section to address http://stackoverflow.com/questions/26638171/how-do-i-create-options-objects-in-scala-js Fix scala-js#152: Document option object pattern Update types.md comments applied remove triple blank lines
@omidb omidb closed this Nov 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants