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

GitHub is deprecating password authentication #1390

Closed
tinchodias opened this issue Nov 4, 2020 · 4 comments
Closed

GitHub is deprecating password authentication #1390

tinchodias opened this issue Nov 4, 2020 · 4 comments

Comments

@tinchodias
Copy link
Collaborator

tinchodias commented Nov 4, 2020

Plain-text credentials for API calls become obsolete soon: https://developer.github.com/changes/2/#--deprecating-password-authentication

We must prepare releases to fix this problem in Pharo 80 and Pharo 90 (at least these 2 versions, but probably there will be users of older Pharo versions that could benefit if it they could load the fix).

With @tesonep, we have implemented token authentication credentials, that fix this issue. We did that in the branch 1.9, which is unstable (it has a partial migration to Spec2 of IceTip-UI). This means that 1.9 can't be directly merged neither in P8 or P9. Let's analyze:

Pharo 80
It should be a minimal hot-fix with since it's the stable version.

Apparently, Iceberg v1.6.7 is the current version here. To get this information, I looked at the history of the Pharo8.0 branch of pharo repo and found this commit and this PR. However, I see there is a tag v1.6.9 (not merged into P8?). I guess we should reimplement the fix for 1.9 in Spec1 and release 1.6.10.

Pharo 90
Iceberg 1.8.3 is used in P9 (pharo-project/pharo#6628). The hot-fix for P8 should work here, rebased in 1.8 branch. In the future, if branch 1.9 is integrated in P9 or P10, it will have the original fix we did with Pablo.

@tinchodias
Copy link
Collaborator Author

About Pharo 8, I've double-checked by downloading a new image to confirm that the version that is merged there is 1.6.7. There are several posterior commits and 2 tags in dev-1.6:

8d6c1f0 (dev-1.6) Merge pull request #1371 from crunsk/patch-2
fe38261 Fix typo in IceTipRepositoryModel.class.st
d15609f (tag: v1.6.9) Merge pull request #1355 from tesonep/fixing-saving-of-classTraits
966dbce Changing tonel to 1.0.17
0b1fe96 - Adding comparison of IceTraitDefinitions to include the classTraitsDefinition - Adding tests
a48ad07 Merge pull request #1348 from sbragagnolo/1345-The-IcePushed-and-IceCommited-announcement-are-never-announced
3679349 (tag: v1.6.8) Remove references to World => self currentWorld
0c5f93d (tag: v1.6.7) Updating Tonel to a new version

@guillep @tesonep :
Where should be the token hot-fix commited?

I'd put it in a new branch that hangs directly from v1.6.7 (avoiding all the listed commits and reducing the impact). But not sure about the new tag name. I'm for a v1.6.10 that doesn't include 1.6.8 and 1.6.9. The other option I considered is naming it "v1.6.7-hotfix" but it breaks the semver spec (discussed here).

@guillep
Copy link
Member

guillep commented Nov 6, 2020

I believe the changes in versions >= 1.6.8 are pharo9 specific, so it should somehow follow 1.6.7 as you say.
I could list other possibilities:

  • name it 1.6.8-pharo8 => It could be confusing with other 1.6.8
  • name it 1.6.7.1 like a patch of a patch version? => this one breaks the structure major.minor.patch but kind of represents what you want better than the others

what do you think?

@tinchodias
Copy link
Collaborator Author

Well, I still think that 1.6.10 is better.

  • the patch of patch is not defined in semantic versioning
  • any "-label" is considered a "pre-release" by semantic versioning, and give this example: 1.0.0-alpha < 1.0.0-alpha.1 < 1.0.0-alpha.beta < 1.0.0-beta < 1.0.0-beta.2 < 1.0.0-beta.11 < 1.0.0-rc.1 < 1.0.0.

Sorry if it's an annoying discussion. Also it depends if we really care on being strict with semantic versioning :p

Maybe, we can additionally merge 1.6.9 with 1.6.10 and tag it 1.6.11, so if anybody loads the latest 1.6 version in Pharo 8, everything will be there...

@guillep
Copy link
Member

guillep commented Nov 6, 2020

To me semantic version is a tool, not a mantra :). We should follow it if it serves us, and otherwise not. I'm not emotionally attached to it, so I don't see any major problems with slightly breaking it in this case. I'd just go with the most practical solution.

Since you pulled out the semver "standard", I'll quote the following parts that seem relevant to me.

What do I do if I accidentally release a backwards incompatible change as a minor version?
As soon as you realize that you’ve broken the Semantic Versioning spec, fix the problem and release a new minor version that corrects the problem and restores backwards compatibility. Even under this circumstance, it is unacceptable to modify versioned releases. If it’s appropriate, document the offending version and inform your users of the problem so that they are aware of the offending version.

What if I inadvertently alter the public API in a way that is not compliant with the version number change (i.e. the code incorrectly introduces a major breaking change in a patch release)?
Use your best judgment. If you have a huge audience that will be drastically impacted by changing the behavior back to what the public API intended, then it may be best to perform a major version release, even though the fix could strictly be considered a patch release. Remember, Semantic Versioning is all about conveying meaning by how the version number changes. If these changes are important to your users, use the version number to inform them.

Anyways, I don't have a strong opinion for one or the other. Whatever change you make (1.6.10 or 1.6.7.1) you are going to break the semantic version contract. So whatever is done it just needs to be properly documented.

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

No branches or pull requests

2 participants