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

Prefix / Suffix support when javaType is not specified #465

Merged
merged 1 commit into from
Dec 29, 2015

Conversation

pwilder
Copy link
Contributor

@pwilder pwilder commented Dec 13, 2015

Please let me know if I need:

  • More tests
  • More comments
  • Style changes
  • Alternate behaviour (e.g. capitlization on nodeName and prefix)

Thanks,

Philip

String fullNodeName = createFullNodeName(nodeName, prefix, suffix);

//Note that only the first letter of the fullNodeName will be capitalized not the node name
//so prefix + nodeName becomes PrefixnodeName
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be consistent, the root name should probably be capitalized. If you define javaType as the value that jsonschema2pojo would pick, the functionality should be identical.

Copy link
Owner

Choose a reason for hiding this comment

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

Agreed. Without capitalizing nodeName, I think the majority of people will be disappointed with how this behaves.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

@ctrimble
Copy link
Collaborator

LGTM. Just needs to be squashed into a single commit.

@joelittlejohn
Copy link
Owner

@pwilder Would you be able to fixup e574960 and 23dae18 into 32b2499?

I'd start with:

git rebase -i 32b2499^

this will allow you to fixup the extra commits.

Let me know if you're having difficult and I can take care of this.

Thanks!!

@pwilder
Copy link
Contributor Author

pwilder commented Dec 21, 2015

I'll give the rebase a try and let you know if I hit issues.

@pwilder
Copy link
Contributor Author

pwilder commented Dec 22, 2015

Squashing on my local side was fairly trivial but the commit log suggests I've made things worse rather then better (5 commits vs. 3). Independently rolling back my local content or the remote content (per http://stackoverflow.com/a/588471) looks manageable but I'm somewhat worried that trying to do both will just lose all my changes (not that they are terribly difficult to reproduce in this case).

@ctrimble
Copy link
Collaborator

@pwilder It looks like you merged changes from master in, instead of replaying history on it.

From where you are at, you can use these steps to create the branch you need:

note: assumes the remote upstream is the main project and that your current checkout is just like pwilder:master.

git checkout -b master-cleanup # make a copy of your branch
git reset --hard HEAD^2        # throw away the last 2 commits
git reset --soft HEAD~2        # restage the 2 commits before that
git commit --amend --no-edit   # amend your first commit
git rebase upstream/master     # replay history on the upstream master

See if the diffs look correct:

git diff upstream/master

And if the history is correct:

git log -2

If everything is good, make this your master

git branch -D master
git branch -m master

Double check where you are at and force push back to your remote master.

@joelittlejohn
Copy link
Owner

@pwilder I think what's happened here, is that you've done the correct squashing locally, but instead of force pushing back to your fork, you've pulled and merged from it instead.

When you rebase those local commits, your local history is basically incompatible with what's in GitHub. At this point, you want to git push -f origin master to force the remote repo to match your local repo. If you pull from it, you'll be keeping all commits that exist locally and remotely and adding merge to reconcile the whole lot.

At this point, the easiest way to fix this is:

git reset --hard d78d121   # reset back to your good, squashed commit
git push -f origin master  # make origin/master match your local repo exactly 

I'm not so worried about rebasing against upstream (I don't mind a branch showing in the history, as long as we can merge without conflicts). @ctrimble is suggesting a process that (for even more bonus points to you) keeps our central log tidy too. Once you've done the above, you can do this very easily by running:

git remote add upstream git@github.com:joelittlejohn/jsonschema2pojo.git
git pull --rebase upstream master
git push -f origin master

If you have any trouble at all or you feel you're getting things into a worse state, just use git reset --hard d78d121 to get back to your good, squashed commit.

@pwilder
Copy link
Contributor Author

pwilder commented Dec 29, 2015

Sorry for the delayed response, holidays have limited my availability.

I started with the instructions from joe and decided to keep things simple and just follow through to the end (with a minor change to use the https url for simplicity). I'm only seeing 1 commit record and the contents of the commit looks good so hopefully this is roughly what we are looking for.

It's been an interesting exercise in git submission discipline that has made clear I could benefit from some more time tinkering with the various git commands. Thank you both for the support.

@joelittlejohn joelittlejohn added this to the 0.4.19 milestone Dec 29, 2015
@joelittlejohn
Copy link
Owner

Thanks @pwilder, glad this was a useful exercise and not an exercise in frustration! 😄

joelittlejohn added a commit that referenced this pull request Dec 29, 2015
Prefix / Suffix support when javaType is not specified
@joelittlejohn joelittlejohn merged commit d105a78 into joelittlejohn:master Dec 29, 2015
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.

3 participants