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

Spelling #104

Merged
merged 31 commits into from May 22, 2017
Merged

Spelling #104

merged 31 commits into from May 22, 2017

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Jan 22, 2017

I've tried to exclude third party code/modules. If I missed a module, please let me know or feel free to exclude it.

@@ -369,7 +369,7 @@ public BindInterceptor setBindListener(BindInterceptor bindListener) {
return setBindInterceptor(bindListener);
}

public BindInterceptor setBindInterceptpr(BindInterceptor bindListener) {
public BindInterceptor setBindInterceptor(BindInterceptor bindListener) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an API break. I can provide a separate PR that includes a deprecation annotation if requested...

@@ -136,7 +136,7 @@ public boolean apply(@Nullable String name) {
/**
* Does a property exist strictly in this class or its ancestors?
*/
/*package*/ final Predicate<String> HAS_PROPERTY_NAME_IN_ANCESTORY = new Predicate<String>() {
/*package*/ final Predicate<String> HAS_PROPERTY_NAME_IN_ANCESTRY = new Predicate<String>() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is an API break

Copy link
Member

Choose a reason for hiding this comment

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

Not an API break since it is package-internal

@@ -86,7 +86,7 @@ public Class onClass(Class c,Void _) {
return c;
}

public Class onParameterizdType(ParameterizedType p,Void _) {
public Class onParameterizedType(ParameterizedType p,Void _) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an API break

@@ -137,7 +137,7 @@ public Type onClass(Class c, Class sup) {
return null;
}

public Type onParameterizdType(ParameterizedType p, Class sup) {
public Type onParameterizedType(ParameterizedType p, Class sup) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an API break

@@ -189,7 +189,7 @@ public Type onClass(Class c, BinderArg args) {
return c;
}

public Type onParameterizdType(ParameterizedType p, BinderArg args) {
public Type onParameterizedType(ParameterizedType p, BinderArg args) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an API break

@@ -22,7 +22,7 @@
<p>
If your view script produces text content, and if you'd like to compress the stream, put <tt>&lt;st:compress xmlns:st="jelly:stapler"></tt> as the root element of your view, like this:
<pre class=code><xmp>
<st:compres xmlns:st="jelly:stapler" ... other ns decls ...>
<st:compress xmlns:st="jelly:stapler" ... other ns decls ...>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seem to be a bug fix afaict

Copy link
Member

Choose a reason for hiding this comment

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

It is

Copy link
Member

Choose a reason for hiding this comment

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

Huh? This is just documentation, not production code.

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

Yes, the API typo fixes need a more graceful changing

@@ -136,7 +136,7 @@ public boolean apply(@Nullable String name) {
/**
* Does a property exist strictly in this class or its ancestors?
*/
/*package*/ final Predicate<String> HAS_PROPERTY_NAME_IN_ANCESTORY = new Predicate<String>() {
/*package*/ final Predicate<String> HAS_PROPERTY_NAME_IN_ANCESTRY = new Predicate<String>() {
Copy link
Member

Choose a reason for hiding this comment

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

Not an API break since it is package-internal

@@ -72,7 +72,7 @@ public final T visit( Type t, P param ) {
}

protected abstract T onClass(Class c, P param);
protected abstract T onParameterizdType(ParameterizedType p, P param);
protected abstract T onParameterizedType(ParameterizedType p, P param);
Copy link
Member

Choose a reason for hiding this comment

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

This is an issue as well since the class is publicly overridable

Copy link
Member

Choose a reason for hiding this comment

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

No, TypeVisitor is not public.

@jsoref
Copy link
Contributor Author

jsoref commented Apr 23, 2017

The API change is now its own PR.

Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

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

👍 LGTM. Maybe the bugfix needs extra investigation, CC @vivek

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

One mistake I will just revert.

@@ -126,7 +126,7 @@ private void processGSstring(Reader reader, StringWriter sw) throws IOException
}

/**
* Closes the currently open write and writes out the following text as a GString expression until it reaches an end %>.
* Closes the currently open write and writes out the following text as a GSstring expression until it reaches an end %>.
Copy link
Member

Choose a reason for hiding this comment

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

No, this was correct before; revert (or, better, switch to @link).

@@ -22,7 +22,7 @@
<p>
If your view script produces text content, and if you'd like to compress the stream, put <tt>&lt;st:compress xmlns:st="jelly:stapler"></tt> as the root element of your view, like this:
<pre class=code><xmp>
<st:compres xmlns:st="jelly:stapler" ... other ns decls ...>
<st:compress xmlns:st="jelly:stapler" ... other ns decls ...>
Copy link
Member

Choose a reason for hiding this comment

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

Huh? This is just documentation, not production code.

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Now OK.

@jglick jglick merged commit 99134f4 into jenkinsci:master May 22, 2017
@jsoref jsoref deleted the spelling branch May 23, 2017 00:33
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

3 participants