Skip to content
This repository has been archived by the owner on Jan 31, 2019. It is now read-only.

FishEye service mods #477

Closed
wants to merge 5 commits into from
Closed

Conversation

MacSwinarski
Copy link
Contributor

  • new property names
  • handling custom repository name correctly
  • handling FishEye URL without protocol, ending slash
  • docs

miszobi and others added 3 commits December 19, 2012 15:20
- Fix FishEye capitalization
- doc field names + tweaks
- make sure service param name matches expected form data
Modified input data names, docs, handling url without protocol, ending slash.
string :url_base, :token, :repository_name
white_list :url_base, :repository_name
string :FishEye_Base_URL, :REST_API_Token, :FishEye_Repository_Name
white_list :FishEye_Base_URL, :FishEye_Repository_Name
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason you changed these? It'll break all old fisheye hooks.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've changed them to get more descriptive field names in the configuration form. matching the ones in FishEye.
Being able to trigger a scan remotely is a new feature, and not available in a released version yet, so breaking existing hooks shouldn't affect anybody.

@miszobi
Copy link
Contributor

miszobi commented Jan 10, 2013

@technoweenie If there aren't any issues, would it be possible to merge this? We'd like to test the integration before the release next week. Thanks!

@technoweenie
Copy link
Contributor

Hooks are on hold for various reasons until after next week. Also, since the keys are changing, we have to update the existing uses of the Hook to use the new filenames. They'll all break if they continue to send url_base, when the hook now looks for FishEye_Base_Url.

@MacSwinarski
Copy link
Contributor Author

@technoweenie
if we get back to old keys would that make merging other changes (bug fixes) easier / faster ?
Thanks!

@technoweenie
Copy link
Contributor

Yes. Changing keys either breaks every hook, or forces me to update them
by hand. However, all updates are currently on hold as I make some
important changes to the GitHub Services backend. Once those are updated,
I'll resume pulling changes back in. I'm hoping I can have this stuff
finished this week, so I can merge the lingering pulls next week.

@MacSwinarski
Copy link
Contributor Author

hi @technoweenie, any idea when this could me merged;

@technoweenie
Copy link
Contributor

Hey, don't worry about merge conflicts. I'll take care of them, since they're due to my refactoring.

We're still in the middle of a big hook backend transition. Once we're on the new backend, I'll make the change. Changing the property names requires some manual work in the DB so everyones existing hooks don't break. That's why this isn't merged yet.

@MacSwinarski
Copy link
Contributor Author

Hey @technoweenie, sorry to bother You but maybe you have some news;)

thx
mac

@MacSwinarski
Copy link
Contributor Author

Hi @technoweenie, any news? let me know how can we make this pull request merged. thx

@technoweenie
Copy link
Contributor

I have to find a time when I can manually migrate existing hooks over to use the new properties. You can revert the property name changes and make my job a lot easier :)

Maciej Swinarski added 2 commits March 19, 2013 18:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants